Memory Leak

[ekisner]

I'm using Visual Leak Detector and numerous calls to the crtdbg, within Visual Studio 2010, trying to find a memory leak.

As best I can tell, I have a calloc that is being called (it will hit the breakpoint that I throw on the calloc). Which means that I must have something wrong somewhere else. Sadly I cannot find this, which means that ultimately it must be a lack of understanding.

Per the following code:

struct Node{
  char text[100];
  Node *next;

  Node(char* text)){
    strcpy(this-text,text);
    next=null;
  }
}
struct List{
  int NodeCount;
  Node *head;

  List(){
    NodeCount=0;
    head=NULL;
  }
  ~List(){
    this->clear();
  }

  void AddToList(char* text){
    if (NodeCount==0)
      head = new Node(text);
    else {
      Node *temp=head;
      while (temp->next!=NULL)
        temp=temp->next;
      temp->next=new Node(text);
    }
  }
  void Clear(){
    Node *temp=this->head;
    for (int i=0;i<NodeCount && temp!=NULL){
      head=head->next;
      delete temp;
      temp=head;
    }
    if (head==NULL)
      delete head;
    NodeCount=0;
  }
}
class Base{
public:
  Base(){
    //Generate array of 5 linked lists.  5 hardcoded for ease of reading.
    list=(List*)calloc(5,sizeof(List));    ///THIS IS WHERE VLD SAYS MY LEAKS ARE COMING FROM///
  }
  ~Base(){
    if (list!=NULL)
      //5 hardcoded for ease of reading.
      for (int i=0;i<5;i++)
        list[i].Clear();
      free(list);
  }

  virtual void Update()=0;


List *list;      /////LIST Member Object.
}

class Child : public Base {
public:
  //actual code has paramters, hence the explicit call of the parent constructor
  Child() : Base() {
    //apply some basic values to the list which will always be valid
    list[0].AddToList("some text from child");
  }

  ~Child(){
    //delete something extra here
  }

  void Update(){
    //Implement pure virtual function here
  }
private:
  //add some extra variables here
}

void main(){
  Base **BunchOfLists;
  //make an array of 3 base/child objects.  3 hardcoded for ease of reading.
  BunchOfLists=(Base**)calloc(3,sizeof(Base*));
  BunchOfLists[0]=new Child();  //actual has parameters.
  BunchOfLists[1]=new Child();
  BunchOfLists[2]=new Child();

  for (int i=0;i<3;i++)
    delete BunchOfLists[i];
  free(BunchOfLists);
} 

Does anyone see where I could be leaking memory? I've indicated in the source where VLD indicates the problem is. In case that comment was missed, it is the Base constructor, where the List object is calloc'd. There is also a leak in the assignment of the head pointer, in AddToList, when NodeCount==0.

In my eyes, I'd have created an array of pointers to Base Objects, each pointer creating a 'new' Base object (which is eventually deleted). Each base object contains a pointer to a List object (which is eventually freed), with an allocated 5 lists which are each Clear()ed.

My theory, is that I'm not correctly adding nodes to the list... that I should be doing something like Node *temp("text"); head=&temp; within my List::AddToList?

[Peter87]

Don't use calloc/malloc/free. They will not call constructors and destructors. Instead use new/delete.

[Texan40]

I'd suggest you either use new/delete or calloc/malloc/free but not both.

like replacing line 48 with:

list = new List[5];

and then in the destructor:

delete [] list;

[ekisner]

Thank you both, such quick answers! I'm in the process of converting my code to use exclusively new/delete. Will post back with the result (success or otherwise)

[Moschops]

This code is full of errors. Does it even compile?

[Peter87]

line 28, shouldn't it be the other way around temp->next = temp;?

[ekisner]

Moschop, I've understood it to be rude to just dump source and say "what's wrong with this"... since my actual source is well over 3,000 lines, I did up a very stripped example right in this text editor (rather than in my IDE). I would be very surprised if it didn't have a few typos in it somewhere. I was hoping that my error was simple, and that a more seasoned programmer would be able to see that I'm simply doing something invalid.

If you would like me to make the code compile, I'd be happy to do so and post that back.

Peter, line 28 is meant to iterate to the end of the list... temp->next=temp would seem to me to be the correct code?

[Peter87]

Yes, I see now. I was wrong.

Yes it is rude to post 3000 lines of code. It is good that you try to make it shorter but make sure it compiles and have the same problem as you describe.

[ekisner]

heheh.... er... yes... >.< That'd be a typo.

I'm still replacing my calloc's with new's, as well as making the example code compile correctly. Will post it back (or post that I was successful) after I've got it all converted/corrected.

[Peter87]

You have forgot to increment NodeCount in AddToList.

[ekisner]

Peter and Texan had it right on their first guess, was definitely the result of my using calloc.

Going to get myself out of that habit! Still have a bunch of other calloc calls to get rid of, but the ones relating to the memory leaks were all switched to new and VLD no longer detects any memory leaks!

Thank you, everyone, for your patience, understanding, and most importantly, your very helpful assistance.

[moorecm]

Try making ~Base virtual. That way deleting Child objects via Base pointer will not leak.

[ekisner]

Thanks moorecm, in my actual code it was virtual already (out of necessity, since the child objects sometimes allocated memory for other items)- but I appreciate the tip.