Reputation: 2663
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)?
Upvotes: 3
Views: 476
Reputation: 17605
To my understanding, if you call free(theName)
before printing the list, the memory pointed to by the last list node is freed. Furthermore I am a bit skeptical about using realloc
to allocate new memory; when printing the list, you might read memory that contains the expected data, but already has been deallocated by realloc
.
Note that realloc is permitted to move the starting address of the memory block, which means that the old content may be still there even when writing to the address returned by realloc
.
Upvotes: 3
Reputation: 748
When you free theName, the pointer still points to the name portion most recent addition to the list. It doesn't point at the earlier items in the list because the pointers are properly being managed by the structure elements, and theName was moved to point at a different value (the most recent addition). This is why the name is being free()d.
You're also leaking memory from not properly freeing up the variables inside each struct element (namely name) before freeing the struct element itself. I would personally recommend getting valgrind (linux) or this (windows) and running your program through it.
Upvotes: 4