0

I wanted to know how to make sure I'm not leaking memory when calling delete on a singly-linked list (implemented by hand, btw).

I created these super basic list,node and person classes to make myself a little bit clearer.

Here you go:

class person {
public:
    person();
    //something to do like sets gets etc...
    virtual ~person();
private:
    int id;
    person* pplArr[5]; //Dynamically allocated person objs.
};
person::person(){
    pplArr[5] = NULL;
}
person::~person(){
    for(int i = 0; i < 5; i++)
        delete pplArr[i];
}

#include "person.h"
class node {
public:
    node(person*, node*);
    person* getData();
    node*   getNext();
    void    setNext(node*);

    virtual ~node();
private:

    person* data;
    node*   next;
};

node::node(person* p, node* n){
    data = p;
    next = n;
}
person* node::getData(){
    return data;
}

node* node::getNext(){
    return next;
}

void node::setNext(node* nxt){
    next = nxt;
}

node::~node(){
    //nothing to delete "manually".
}

#include "node.h"

class list {
public:
    list();

    node* getFirst();
    void  insert(person*);

    virtual ~list();
private:

    node* first;
};

node* list::getFirst(){
    return first;
}
void list::insert(person* p){
    if(first){
        first->setNext(new node(p, NULL));
    }
    else {
        first = new node(p, NULL);
    }
}
list::~list(){
    node* aux;
    while (first){
        aux = first->getNext();
        delete first;
        first = aux;
    }
}

Okay, so as you can see we have these 3 classes:

person contains an array of 5 people objects, dynamically allocated:

node contains a pointer to next node, and data which contains the actual person object. and,

list that contains the nodes and manages them.

In order to successfully deallocate all the memory, I need call delete listName, this line will go into each node and call delete for each node, which will in itself call the delete for person.

The delete for person will go into each array slot and call the 'delete' of those persons to release that memory.

After that, it will execute the other deletes that are waiting. From bottom to top.

Is this correct? Are my destructors correct?

I would just like to know how to completely release the memory I allocated if there is a singly-linked list that contains nodes that have objects that have dynamically allocated memory.

I'm very confused, my apologies if this is nonsense or utterly bad implementation.

PS: I don't know if this list works, I just wanted to make a basic linked list to see if you guys could help me understand, so hopefully I can grasp the concept on actual, more complex lists. I am aware that there are linked lists and other data structures readily available on libraries but college teachers ask us to do it this way first.

10110
  • 2,353
  • 1
  • 21
  • 37
  • 2
    Don't use `new` `delete` yourself please. Just use [smart pointers](http://en.cppreference.com/w/cpp/memory) instead. – πάντα ῥεῖ May 20 '17 at 14:19
  • 3
    OT: `pplArr[5] = NULL;` is getting out of the bound of the array. – songyuanyao May 20 '17 at 14:20
  • Thanks! I understand, the thing here is that my teacher ask us to do it this way to learn the language and the inner workings before using auto stuff. :) – 10110 May 20 '17 at 14:21
  • I know there are bugs, I just wanted to do an sketch of a simple list so you could help me understand how to release all the memory, I am aware this actual code won't behave correctly. I am interested in the destructors solely and how to release that memory – 10110 May 20 '17 at 14:23
  • Seems fine. Look at: http://stackoverflow.com/questions/2814188/c-array-of-pointers-delete-or-delete – alejandrogiron May 20 '17 at 14:25
  • 1
    This question would be more suited on https://codereview.stackexchange.com/ – Mr Lister May 20 '17 at 14:29
  • @DanielSegura _"... the thing here is that my teacher ask us to do it this way ..."_ Please point your teacher [here](http://dev-jungle.blogspot.de/2015/02/i-have-dream-im-dreaming-of-so-called-c.html?spref=bl). – πάντα ῥεῖ May 20 '17 at 14:30
  • There is tools to check if there is a leak: ``valgrind``, the ``sanitizer`` from clang (and gcc) ... But you should notice something in your code: you have no way of knowing who is the owner of the owner the resource. That's why you should check out RAII and smart pointer, ... (if you can't use STL smart pointer, ``unique_ptr`` (for not array type) is quite easy to implement). – nefas May 20 '17 at 14:32
  • You have more worries than just memory leaks. How about `delete`ing an uninitialized pointer, resulting in undefined behavior and a nearly guaranteed crash? You haven't even tried running your code, haven't you? – Sam Varshavchik May 20 '17 at 14:35
  • Typical C++ of teachers who learnt it 20 years ago and never bothered to get the knowledge of their subject updated :( @DanielSegura> when you're done with your exercise, get a recent c++ introductory book/online course and avoid learning the outdated c++ from last century ^^ – spectras May 20 '17 at 14:45
  • 1
    The constructor exhibits undefined behaviour by setting `pplArr[5] = NULL`, since valid indices of an array with 5 elements are `0` to `4`. The destructor also exhibits undefined behaviour, since the constructor does not initialise `pplArr[0]` through `pplArr[4]`, no other code you've shown does either, and the destructor uses operator `delete` on each one. So, while there is probably no memory leak, correct behaviour of your program is not guaranteed either. – Peter May 20 '17 at 14:55
  • I just had a job interview for a major company that required C style strings so let's not dump them just yet. One can indeed dream though. – didiz May 20 '17 at 16:30

0 Answers0