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 realloc
s 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)?