C++ vector of class objects and dynamic memory allocation

Question

This is a simplified version of what I want to do, it crushes when push_back is called, specifically when the destructor is called. If I delete the body of the destructor it works, but I want to be sure that it is deleted. Instead of calloc/free I tried new/delete with the same result. What do I do wrong here?

#include <cstdlib>
#include <vector>


using namespace std;

class Buffer
{
public:
    float *DATA;

    Buffer(int length) {DATA = (float*) calloc (length,sizeof(float));};
    virtual ~Buffer(){free (DATA);};
    
private:
};

int main(int argc, char** argv)
{
    vector <Buffer> v;
    for (int i =0; i<10; i++)
        v.push_back(Buffer(1000));
    
    return 0;
}

`Buffer` is not [rule-of-three](https://en.cppreference.com/w/cpp/language/rule_of_three) compliant. It needs to be. What was your intent with `DATA = buffer.DATA` ? You realize all that does is wire two `Buffer` objects `DATA` members to the *same* dynamic pointer value? So.. who actually *owns* that thing? I seriously doubt that is intentional, and if not, simplest way: `std::vector DATA;` and throw the rest of that manual dynamic madness away. — WhozCraig, Jan 31 '21 at 20:07
thank you for your comment, I deleted copy constructor, but issue is still there, the error is "double free or corruption (!prev)" — SVS, Jan 31 '21 at 20:19
That's because the default copy-ctor does *exactly* what yours did. Did you *read* the page at [the link I provided](https://en.cppreference.com/w/cpp/language/rule_of_three), and then consider how important it is? Consider the opening sentence: *"If a class requires a user-defined destructor, a user-defined copy constructor, or a user-defined copy assignment operator, it almost certainly requires all three."* - Well, you have a custom destructor, so you likely need a *proper* custom copy-ctor and copy-assignment operator. — WhozCraig, Jan 31 '21 at 20:20
thank you, i did, i am trying to comprehend that information, it is not obvious, would you be able to provide an example for my case — SVS, Jan 31 '21 at 20:23
@SVS The “you need all three” admonition is all you need to know. Write all three (correctly). Problem solved. If you don’t know how to write those other methods/constructors, ask specific questions and make sure you got some code or at least a description of proposed approach to start with. — Kuba hasn't forgotten Monica, Jan 31 '21 at 21:06

score 1 · Accepted Answer · answered Jan 31 '21 at 20:22

1

Here's a working code: https://godbolt.org/z/ex9oMG.

#include <cstdlib>
#include <vector>

using namespace std;

class Buffer
{
public:
    float *DATA;

    Buffer(int length) {DATA = (float*) calloc (length,sizeof(float));};
    Buffer(const Buffer &buffer) = delete;
    Buffer(Buffer&& buffer) { 
        DATA = buffer.DATA;
        buffer.DATA = nullptr;
    }
    ~Buffer(){
        if (DATA) free(DATA);
    };
    
private:
};

int main(int argc, char** argv)
{
    vector <Buffer> v;
    for (int i =0; i<10; i++)
        v.push_back(Buffer(1000));
    return 0;
}

You need to define a move constructor here, primarily because v.push_back(Buffer(1000)) requires a move op otherwise the deletion of the original copy would free the resource.

I've explicitly deleted the copy ctor because when handling such resources - copy should not be allowed.

answered Jan 31 '21 at 20:22

theWiseBro

1,439
12
11

Reinventing `std::unique_ptr` notwithstanding, `vector v2; v2 = v;` = no-joy ? Why such a restriction ? I.e. *"when handling such resources - copy should not be allowed."* - why ?? – WhozCraig Jan 31 '21 at 20:30
1

@WhozCraig well OP has presented a "simplified" version of the actual problem he's facing - there probably is a reason why he's not using smart pointers. Whether copy should be allowed or not - well we'll just be dealing with reinventing `std::shared_ptr` in that case :/ – theWiseBro Jan 31 '21 at 20:34
As we don't know what the use is, I'm not convinced a simple `std::vector` and outright removal of `Buffer` entirely isn't applicable to the real need X of this probable XY-problem, especially considering retention of the *magnitude* of the allocation within `Buffer` isn't even done; that itself is a significant code aroma. – WhozCraig Jan 31 '21 at 21:00
As said, it is a simplified example, extracted from 1000s lines of code used for processing satellite data (100 GBs). Buffer contains functions that read/write data to DATA in real time, it also contains variety of other variables that describe the object... I guess my original mistake was not to delete the reference after copying, i.e. "buffer.DATA = nullptr;". is this correct? – SVS Jan 31 '21 at 23:17
@SVS Not exactly. You were using a copy ctor. A copy ctor is not meant to transfer resources - that is the role of the move ctor. `buffer.DATA = nullptr` should only be part of a move ctor. If you truly want to share the resource(DATA), you need a proper resource ownership management - something similar to what `std::shared_ptr` does. – theWiseBro Feb 01 '21 at 05:22

C++ vector of class objects and dynamic memory allocation

1 Answers1

Linked