0

If I declare class MyObj like so:

class MyObj: {
  private:
    uint8_t *arrayPtr;
  public:
    void makeArray();
}

void MyObj::makeArray() {
  arrayPtr = new uint8_t [10];
}

Then I call:

void func() {
  MyObj testObj;
  testObj.makeArray();
}

Now that func has run and completed and testObj can be forgotten about, does the 10 byte array get removed from the stack? Or do I need to create an explicit destructor for MyObj that tests for the existence of the array and calls the delete function on it?

Bo Thompson
  • 359
  • 1
  • 12
  • 1
    Yes, you need do `delete` something. Rule of thumb, no exceptions, 100% guarantee: if you `new` something, it must be deleted exactly once, at some point. The End. – Sam Varshavchik Apr 11 '20 at 21:39
  • Edited for clarity in response to @cigien. – Bo Thompson Apr 11 '20 at 21:40
  • It would be rather difficult and error-prone to implement this. The compiler would need to be sure it's pointing to dynamically-allocated memory, know whether to use `delete` or `delete[]`, and know that this object is the sole (strong) owner of that memory block. Any uncertainty in any of these areas would cause improper deletes, arguably more harmful than the memory leak with the do-nothing default. It would also fail to handle any non-memory resources. – chris Apr 11 '20 at 21:42

2 Answers2

3

You may not call delete in this code snippet

MyObj testObj;
testObj.makeArray();
delete testObj;

because the testObj is not a pointer that was assigned by the address of memory that was allocated with using the operator new.

You need to add to the class definition at least an initializer and destructor

class MyObj {
  private:
    uint8_t *arrayPtr = nullptr;
  public:
    ~MyObj() { delete [] arrayPtr; }
    void makeArray();
};

Pay attention to that either you should define the copy constructor and the copy assignment operator as deleted or you have to explicitly define them.

Also bear in mind that the function makeArray is unsafe. If the user will call it the second time there will be a memory leak because the previously allocated memory will not be deleted.

And you do not have an array as a data member pf the class. You have a pointer. That pointer will be freed from the stack together with the object containing the pointer. However the dynamically allocated array will not be freed without calling the operator delete [].

Vlad from Moscow
  • 301,070
  • 26
  • 186
  • 335
  • That destructor doesn't seem to care how many elements are in the array pointed to by arrayPtr. Does the destructor just know how many elements are there by itself? – Bo Thompson Apr 11 '20 at 21:42
  • Please dont break the rule of 0/3/5 even in snippets – Mike Vine Apr 11 '20 at 21:42
  • @BoThompson This information is stored when the memory is allocated and a pointer to the allocated memory is returned. – Vlad from Moscow Apr 11 '20 at 21:44
  • @MikeVineThe question is specific relative to the delete operator. There is no need to touch another question. – Vlad from Moscow Apr 11 '20 at 21:45
  • Hold on, my question isn't specific to the delete operator; the initial edit of my question had a bad example. I've re-worded it to contain no explicit delete operation. Is your answer still valid in this new case? – Bo Thompson Apr 11 '20 at 21:48
  • @BoThompson My answer is correct. What was allocated in the heap using the operator new must be deleted using the operator delete. – Vlad from Moscow Apr 11 '20 at 21:51
  • @BoThompson, The new wording is confusing. There is no 10-byte array on the stack if you use `new` to allocate it (with the default allocator). In order to properly clean up, the generated destructor would still need to `delete[]` it. – chris Apr 11 '20 at 21:52
  • Thank you for pointing out that makeArray was unsafe. I can see that I should be testing to see if arrayPtr returns nullptr and perform delete [] and such. For posterity, could you include the modification to makeArray() in your answer? – Bo Thompson Apr 11 '20 at 22:00
1

You are not allowed to call delete on an object with automatic storage duration ("on the stack"). It causes undefined behavior of the program.

You must call delete (or delete[]) on exactly the pointer value returned from new (or new[]) and this is never done for you automatically.

So yes, you need to add a destructor that does the deletion and you must be careful to follow the rule-of-0/3/5, meaning you will need to write copy constructor and assignment operators with correct semantics as well.

You can avoid all the trouble by using std::vector<uint8_t> for arrayPtr instead.

walnut
  • 21,629
  • 4
  • 23
  • 59