Jamie Dale
Jamie Dale

Reputation: 69

Can anyone see why this program generates a segmentation fault

I am writing a program to implement a map using a singly linked list. After writing and including this insert method the program generates a segmentation fault but i'm not sure where this comes from.

int map_insert(Map *theMap, char *theKey, void *theItem){

   node *newNode = malloc(sizeof(node));

   node *cursor = theMap->root;

   while(cursor->next !=  NULL){
      cursor = cursor->next;
    }

    newNode->key = theKey;
    newNode->item = theItem;
    newNode->next = NULL;


    cursor->next = newNode;

    return (node *)newNode;
}

Upvotes: 1

Views: 77

Answers (2)

mtijanic
mtijanic

Reputation: 2902

node *cursor = theMap->root;

I'm assuming if the map is empty, root will be NULL.

while(cursor->next != NULL)

If root was NULL, cursor is NULL as well, and you are dereferencing it when accessing the next field.

Perhaps change the while condition to:

while (cursor && cursor->next) ?


EDIT: Here's a full function that works:

node * map_insert(Map *theMap, char *theKey, void *theItem){

    node *newNode = malloc(sizeof(node));

    newNode->key = theKey;
    newNode->item = theItem;
    newNode->next = NULL;

    node *cursor = theMap->root;

    if (cursor) {
       while(cursor->next !=  NULL){
          cursor = cursor->next;
        }
        cursor->next = newNode;
    }
    else
    {
        theMap->root = newNode;
    }

    return newNode;
}

Upvotes: 1

Spikatrix
Spikatrix

Reputation: 20244

The signature of the function map_insert is

int map_insert(Map *theMap, char *theKey, void *theItem)

As you can see, it is designed to return an int. But you return a node*. Fix the problem by changing it to:

node* map_insert(Map *theMap, char *theKey, void *theItem){


The cast here:

return (node *)newNode;

is not required as newNode is already of type node*.

Upvotes: 1

Related Questions