0

I have an issue when it comes to preventing memory leaks in my program.


#include <iostream>

using namespace std;

class String
{
    public:
        String();
        String(char str[], int size);
        ~String();
        void clear();
    private:
        char *data;
        int size;
};

String::String()
{
    size = 0; 
    data = NULL;
}

String::String(char str[], int _size)
{
    data = new char[_size];
    for(int i=0;i<_size;i++)
        data[i] = str[i];
    size = _size;
}

String::~String() 
{
  //  if(data != NULL)
//        delete [] data;
}

void String::clear()
{
    if(data != NULL)
        delete []data;
    size = 0;
}

int main()
{
    char temp[] = {'a','b','c','d'};
    String str(temp, 4);
    str.clear();
}

In this code, I have no memory leaks, but it requires me to manualy deallocate memory by calling the clear method. If I instead uncomment the code within the destructor and delete the str.clear() line, I get an error saying that I am freeing the same memory twice. I am confused because the clear method and destructor contains practially the same code, yet such different results.

CuberBoy
  • 15
  • 3
  • 1
    set data to null in the clear method – pm100 Oct 31 '22 at 00:59
  • you have a check `if(data != NULL) delete []data;` but nowhere do you set the `data = nullptr`. You should do this in clear, then it should work. – Fantastic Mr Fox Oct 31 '22 at 01:01
  • Sorry if I wasnt clear, I wanted to be able to not call clear() at all, and simply let the destructor do it all – CuberBoy Oct 31 '22 at 01:10
  • 1
    @CuberBoy if you simply remove the call to `clear()` in `main()`, and uncomment the `delete[]` in `~String()`, then the code shown will work fine (BTW, the null check in `~String()` is redundant and can be removed, as `delete[]` handles that for you). What you claim about double-freeing memory is wrong, unless this is not your real code. However, before you can use this `String` class for general purpose use, make sure to update it to follow the [Rule of 3/5/0](https://en.cppreference.com/w/cpp/language/rule_of_three) to avoid future problems – Remy Lebeau Oct 31 '22 at 01:26
  • 2
    The code shown, with `clear` call removed and the body of destructor uncommented, does not free memory twice. I bet the code you are actually running is different from that shown - and in particular, it involves a copy of `String`. That would indeed lead to double-destruction, because your class violates [The Rule of Three](https://stackoverflow.com/questions/4172722/what-is-the-rule-of-three) – Igor Tandetnik Oct 31 '22 at 01:26
  • @CuberBoy *I have an issue when it comes to preventing memory leaks in my program.* -- Go to the [Rule of 3](https://stackoverflow.com/questions/4172722/what-is-the-rule-of-three), and then scroll to the **Managing resources** section of that page. Does that code look familiar? It looks a lot like your code, doesn't it? – PaulMcKenzie Oct 31 '22 at 01:53
  • @CuberBoy `if(data != NULL); delete [] data;` -- There is no need to do a null check when issuing a `delete[]` call. The destructor should simply be `{ delete [] data; }`. Also, it should not be commented out, as the issue is elsewhere -- leave it uncommented and actually fix the issue. The destructor not "working" is telling you that the other parts of your class are wrong (in this case, missing, like a user-defined copy constructor and assignment operator). – PaulMcKenzie Oct 31 '22 at 02:07
  • If the call to `str.clear()` gets removed, and the code in the destructor gets uncommented, the shown code runs without issues. It is only when `str.clear()` gets called ***and*** the code in the destructor is uncommented, then you get a double-free, due to the obvious bug in `clear()`. VTC as a typo. – Sam Varshavchik Oct 31 '22 at 02:22
  • A few random off-topic notes: (1) “`using namespace std;`” Please remove this. (2) Use `std::unique_ptr data`. That will solve all your problems. (3) Do not assign instance members in a constructor body, ever; an initializer list must do that. Your future self will thank you. – Andrej Podzimek Oct 31 '22 at 03:29

0 Answers0