Reputation: 328
I´m desperate because this code form time to time gives me a segmentation fault and I have no clue why. Actually it´s only supposed to add some linked list notes, print them and then empty the list by freeing the memory.
struct int_list {
int value;
struct int_list *next;
};
typedef struct int_list IntList;
void list_print(IntList *start)
{
IntList *cur = start;
while(cur != NULL)
{
printf("%d\n", cur->value);
cur = cur->next;
}
}
void list_append(IntList **start, int newval)
{
IntList *newel = malloc(sizeof(IntList));
newel->value = newval;
newel->next = NULL;
if(*start == NULL)
{
*start = newel;
}
else
{
IntList *cur = *start;
while(cur->next != NULL)
{
cur = cur->next;
}
cur->next = newel;
}
}
void list_free(IntList *start)
{
IntList *prev = start; // prev = start
while (start != NULL) // if start != Null
{
start = start->next; // make start point to the next element
printf("Deleting %d\n", prev->value);
free(prev); // delete the previous element
prev = start; // make previous point to start again
}
printf("\n");
}
int main(int argc, char *argv[])
{
// fill the list
IntList *start = NULL;
list_append(&start, 42);
list_append(&start, 30);
list_append(&start, 16);
// print the list
printf("\nList 1\n");
list_print(start);
printf("\n");
// free the memory and print again
list_free(start);
printf("Empty list:\n");
list_print(start);
printf("\n");
}
Everything was working just fine before I tried to implement list_free(). So I strongly assume the error can be found in this function. Just posting the rest of the code as well because I´m new to structures and am not 100% sure about having handles them correctly. Do you know what I´m doing wrong?...
Upvotes: 5
Views: 96
Reputation: 1
Pointer is still pointing to the memory location of the deallocated memory, which is also an instance of segmentation fault. It is "undefined behavior" and can cause arbitrarily unpredictable things to happen as the contents of pointed location are unknown, varying runtime.
Upvotes: 0
Reputation: 310980
The function list_free
gets its argument by value. So the function deals with a copy of the original pointer to node. As a result the original pointer to node start
stays unchanged.
And as a consequence the output of the list after calling the function list_free
list_free(start);
printf("Empty list:\n");
list_print(start);
has undefined behavior.
The function should accept the original pointer to node by reference as the function list_append
does.
For example
void list_free( IntList **start )
{
while ( *start != NULL )
{
IntList *prev = *start; // prev = start
*start = ( *start )->next; // make start point to the next element
printf("Deleting %d\n", prev->value);
free(prev); // delete the previous element
}
printf("\n");
}
Call the function like
list_free( &start );
After exiting the function the original pointer start
will be equal to NULL
. That is the list will be indeed freed.
This is better than when the client of the list shall explicitly set the pointer to NULL
himself. He can make the same error as you did forgetting to set the pointer to NULL.
Upvotes: 1
Reputation: 12732
You have undefined behavior because of dangling pointer
list_free(start);
That is, start
is still pointing to freed memory which you are trying to access.
You need to set start
to NULL
after free
ing.
list_free(start);
start = NULL;
printf("Empty list:\n");
list_print(start);
Upvotes: 4