CODEWITHSUNDEEP
c++memory-leaksnew-operatordynamic-memory-allocationdelete-operator
Josh Brown
Josh Brown

Reputation: 21

Why do I get memory leaks while properly deallocating memory?

That's all the code with allocation and deallocation

Constructor

 Event::Event(){
    setEmpty();
}

Destructor

    Event::~Event(){
    delete [] event_description;
}

Copy constructor

Event::Event(const Event& Event){
      if (Event.event_description == nullptr) {
          event_description = nullptr;

      } else {
           event_description = new char[strlen(Event.event_description)+1];
          strcpy(event_description, Event.event_description);

      }
    time_in_sec = Event.time_in_sec;
}

Copy assignment operator

Event& Event::operator=(const Event& Event){
    delete [] event_description;
    if (Event.event_description == nullptr) {
        event_description = nullptr;

    } else {
         event_description = new char[strlen(Event.event_description)+1];
        strcpy(event_description, Event.event_description);
    }


 time_in_sec = Event.time_in_sec;

    return *this;
}

Function that sets description in further development. The main task is to dynamically allocate memory for the event description. I get memory leaks while checking through valgrind

void Event::setDescription(const char* new_desc){
    if (new_desc == nullptr || new_desc[0] == '\0') {

        event_description = nullptr;
        time_in_sec = 0;
    } else {
        event_description = new char[strlen(new_desc)+1];
        strcpy(event_description, new_desc);
        time_in_sec = g_sysClock;
    }
}

Upvotes: 2

Views: 86

Answers (1)

Vlad from Moscow
Vlad from Moscow

Reputation: 311196

This function

void Event::setDescription(const char* new_desc){
    if (new_desc == nullptr || new_desc[0] == '\0') {

        event_description = nullptr;
        time_in_sec = 0;
    } else {
        event_description = new char[strlen(new_desc)+1];
        strcpy(event_description, new_desc);
        time_in_sec = g_sysClock;
    }
}

produces memory leaks. It does not delete the previous allocated memory the address to which is stored in the data member event_description.

It should be defined at least like

void Event::setDescription(const char* new_desc){
    delete [] event_description;   

    if (new_desc == nullptr || new_desc[0] == '\0') {
        event_description = nullptr;
        time_in_sec = 0;
    } else {
        event_description = new char[strlen(new_desc)+1];
        strcpy(event_description, new_desc);
        time_in_sec = g_sysClock;
    }
}

A more safer approach can look like

void Event::setDescription(const char* new_desc){
    if (new_desc == nullptr || new_desc[0] == '\0') {
        delete [] event_description;   
        event_description = nullptr;
        time_in_sec = 0;
    } else {
        char *tmp = new char[strlen(new_desc)+1];
        delete [] event_description;
        event_description = tmp;
        strcpy(event_description, new_desc);
        time_in_sec = g_sysClock;
    }
}

Upvotes: 2

Related Questions