How can I know if I have successfully freed the whole array of singly linked lists?

Question

I have created an array of pointers. I have used some of them as roots for singly linked lists. The size of lgroup(the array of pointers) is 10 and j can get up to 10 as well. I malloc every lptr like this:

lgroup[i].lptr[j] = (node *) malloc (sizeof(node));

and if I'm not going to use it I set curr = lgroup[i].jptr[j]; and curr->next = NULL;. Otherwise I just start using malloc on every curr->next and then I do curr = curr->next;. When I don't want to add any more nodes to a list I just put curr->next = NULL; and move on to the next lptr. Pretty standard stuff.

At the end of my program I want to free all of the memory that I have claimed for my lists. This is how I try to do it:

for (i = 0; i < 10; i++) {
  for (j = 0; j < 10; j++) {
    if (lgroup[i].lptr[j] == NULL) {
      free(lgroup[i].lptr[j]);
      continue;
    } else {
      curr = lgroup[i].lptr[j];
      while(curr->next != NULL) {
        curr2 = curr->next;
        free(curr);
        curr = curr2;
      }
    }
  }
}

The thing is that I ended up with this code after a lot of trial and error and a lot of messages of "double freeing" and other stuff like that, so I'm not completely sure if it actually frees all the memory that I have claimed or that it just happens to compile and run without error but not doing all the things I want it to. I tried to run it on gdb but I really can't understand a lot of things by looking at memory addresses so I was curious if there is a way to check if it works as I expected or if I'm doomed to do it using pen and paper and running the code in my head. If it happens to actually do what it was designed for, would there be an easier and cleaner way to achieve the same results?

Thank you in advance and if you need any clarification don't hesitate to ask for it in the comments.

Note: [you should prefer](http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc) `x = malloc(sizeof(*x))` to `x = (T *)malloc(sizeof(T))`. — Oliver Charlesworth, Jan 05 '12 at 18:42
Thanks alot. I had no idea such a small thing could have so much impact. — Eternal_Light, Jan 05 '12 at 19:27

score 6 · Answer 1 · answered Jan 05 '12 at 18:40

6

Run your code under Valgrind. This is a memory profiler which does many things, but one of the things it does is check for unreleased memory at the end of an application run.

answered Jan 05 '12 at 18:40

Oliver Charlesworth

267,707
33
569
680

Thanks a lot! I just read the quick start guide and it looks like it is a pretty easy tool to use :) Also looks like I have it already installed on my computer?? It might be part of some dev package. – Eternal_Light Jan 05 '12 at 19:05
Umm Valgrind shows "definitely lost" messages on mallocs that I have freed for sure. Would the way I call malloc have anything to do with it? – Eternal_Light Jan 05 '12 at 19:20
@Eternal_Light: If you're talking about my comment above, then no, this shouldn't affect what Valgrind reports. Can you create a minimal test-case? – Oliver Charlesworth Jan 05 '12 at 19:30
You mean a clean c file where i will just malloc and free something? – Eternal_Light Jan 05 '12 at 19:32
@Eternal_Light: Yes, and then make it progressively more and more complicated until Valgrind starts reporting strange things. Alternatively, take your application and start progressively simplifying it until the strange reports stop happening. – Oliver Charlesworth Jan 05 '12 at 19:34
Anyway, thanks a lot. I have trouble understanding why my program is leaking from places it shouldn't but I believe that, following your advice, I will solve the mystery. – Eternal_Light Jan 05 '12 at 19:46

igorrs · Accepted Answer · 2012-01-06T02:15:02.177

2

The first call to free in the code you have posted does not have any effect, since the argument is a NULL pointer (using continue in that context is also unnecessary).

It also seems you are not calling free for lgroup[i].lptr[j] when it is not NULL and lgroup[i].lptr[j]->next is NULL.

I would rewrite it to:

for (i = 0; i < 10; i++) {
    for (j = 0; j < 10; j++) {
        curr = lgroup[i].lptr[j];
        while (curr != NULL) {
            curr2 = curr->next;
            free(curr);
            curr = curr2;
        }
    }
}

edited Jan 06 '12 at 02:15

answered Jan 06 '12 at 02:00

igorrs

350
1
7

So I would be alright if I had checked for lgroup[i].lptr[j]->next == NULL in my first if statement and freed that inside it? – Eternal_Light Jan 06 '12 at 10:43
@Eternal_Light It would be better, but the leak would still be there (consider the case where neither `lgroup[i].lptr[j]` nor `lgroup[i].lptr[j]->next` is NULL, but `lgroup[i].lptr[j]->next->next` is). The only important problem is that the `while` was checking the wrong condition. – igorrs Jan 06 '12 at 18:55

How can I know if I have successfully freed the whole array of singly linked lists?

2 Answers2