Question

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

Was it helpful?

Solution

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.

OTHER TIPS

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.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top