While working with linked lists in C I noticed this behavior that I don't understand. The example code below illustrates the situation where a simple list is declared and filled with nodes that contain *char names. theName string is generated by appending each of the parameters given in the command line by _, so charNum is greater than argv[i] by 2 to accommodate _ and \0. Each of argv elements generates a node that is added to the list in the for loop in the main function.
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
struct node {
char* name;
struct node* next;
};
struct node*
nalloc(char* name)
{
struct node* n = (struct node*) malloc(sizeof(struct node));
if (n)
{
n->name = name;
n->next = NULL;
}
return n;
}
struct node*
nadd(struct node* head, char* name)
{
struct node* new = nalloc(name);
if (new == NULL) return head;
new->next = head;
return new;
}
void
nprint(struct node* head)
{
struct node* n = NULL;
printf("List start: \n");
for(n = head; n; n=n->next)
{
printf(" Node name: %s, next node: %p\n", n->name, n->next);
}
printf("List end. \n");
}
void
nfree(struct node* head)
{
struct node* n = NULL;
printf("Freeing up the list: \n");
while (head)
{
n = head;
printf(" Freeing: %s\n", head->name);
head = head->next;
free(n);
}
printf("Done.\n");
}
int
main(int argc, char** argv)
{
struct node* list = NULL;
char* theName = (char*) malloc(0);
int i, charNum;
for (i=0; i < argc; i++)
{
charNum = strlen(argv[i]) + 2;
theName = (char*) realloc(NULL, sizeof (char)*charNum);
snprintf(theName, charNum, "%s_", argv[i]);
list = nadd(list, theName);
}
nprint(list);
nfree(list);
free(theName);
return 0;
}
The code above works as one would expect:
$ ./a.out one two three
List start:
Node name: three_, next node: 0x1dae0d0
Node name: two_, next node: 0x1dae090
Node name: one_, next node: 0x1dae050
Node name: ./a.out_, next node: (nil)
List end.
Freeing up the list:
Freeing: three_
Freeing: two_
Freeing: one_
Freeing: ./a.out_
Done.
However when I modify this code and call free(theName) before printing the list:
...
free(theName);
nprint(list);
nfree(list);
return 0;
...
the name of the last list item is missing:
$ ./a.out one two three
List start:
Node name: , next node: 0x3f270d0
Node name: two_, next node: 0x3f27090
Node name: one_, next node: 0x3f27050
Node name: ./a.out_, next node: (nil)
List end.
Freeing up the list:
Freeing:
Freeing: two_
Freeing: one_
Freeing: ./a.out_
Done.
So freeing up theName pointer affected the list node that was using it as its name, but how come earlier reallocs didn't affect the other nodes? If free(theName) broke the last node's name I would guess that realloc would do the same and all nodes from the list would have blank names.
Thank you all for your comments and answers. I modified the code to remove casting of malloc's results, add freeing of node->name and change 'malloc -> multiple reallocs -> free' to 'multiple mallocs -> free' for the name. So here's the new code:
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
struct node {
char* name;
struct node* next;
};
struct node*
nalloc(char* name)
{
struct node* n = malloc(sizeof(struct node));
if (n)
{
n->name = name;
n->next = NULL;
}
return n;
}
struct node*
nadd(struct node* head, char* name)
{
struct node* new = nalloc(name);
if (new == NULL) return head;
new->next = head;
return new;
}
void
nprint(struct node* head)
{
struct node* n = NULL;
printf("List start: \n");
for(n = head; n; n=n->next)
{
printf(" Node name: %s, next node: %p\n", n->name, n->next);
}
printf("List end. \n");
}
void
nfree(struct node* head)
{
struct node* n = NULL;
printf("Freeing up the list: \n");
while (head)
{
n = head;
printf(" Freeing: %s\n", head->name);
head = head->next;
free(n->name);
free(n);
}
printf("Done.\n");
}
int
main(int argc, char** argv)
{
struct node* list = NULL;
char* theName;
int i, charNum;
for (i=0; i < argc; i++)
{
charNum = strlen(argv[i]) + 2;
theName = malloc(sizeof (char)*charNum);
snprintf(theName, charNum, "%s_", argv[i]);
list = nadd(list, theName);
}
nprint(list);
nfree(list);
free(theName);
return 0;
}
The above works as expected:
$ ./a.out one two three
List start:
Node name: three_, next node: 0x1826c0b0
Node name: two_, next node: 0x1826c070
Node name: one_, next node: 0x1826c030
Node name: ./a.out_, next node: (nil)
List end.
Freeing up the list:
Freeing: three_
Freeing: two_
Freeing: one_
Freeing: ./a.out_
Done.
However when I place free(theName); before nprint(list);:
free(theName);
nprint(list);
nfree(list);
return 0;
In the output the last node's name is missing and nfree(list); throws error:
$ ./a.out one two three
List start:
Node name: , next node: 0x1cf3e0b0
Node name: two_, next node: 0x1cf3e070
Node name: one_, next node: 0x1cf3e030
Node name: ./a.out_, next node: (nil)
List end.
Freeing up the list:
Freeing:
*** glibc detected *** ./a.out: double free or corruption (fasttop): 0x000000001cf3e0d0 ***
======= Backtrace: =========
...
======= Memory map: ========
...
Aborted
When I put free(theName); after nprint(list); and before nfree(list);:
nprint(list);
free(theName);
nfree(list);
return 0;
in the output all nodes are printed properly, but nprint(list); still throws the error:
$ ./a.out one two three
List start:
Node name: three_, next node: 0x19d160b0
Node name: two_, next node: 0x19d16070
Node name: one_, next node: 0x19d16030
Node name: ./a.out_, next node: (nil)
List end.
Freeing up the list:
Freeing:
*** glibc detected *** ./a.out: double free or corruption (fasttop): 0x000000001cf3e0d0 ***
======= Backtrace: =========
...
======= Memory map: ========
...
Aborted
This raises another question in my mind: I'm guessing that in any case the memory pointed by theName is freed twice: first as node->name and second as theName, so how come free(theName); does not raise the double-free error when called at the end of the program after nfree(list); (as it is in the working code)?
