CODEWITHSUNDEEP
cdata-structuresstructlinked-listsingly-linked-list
breakthatbass
breakthatbass

Reputation: 376

Can't figure out why I am getting a segmentation fault when I try to append to the end of a linked list

I'm trying to practice and get comfortable with linked lists so i made this program to try to create nodes and add to the end of a linked list as it takes in data.

Everything work until it gets to the add_node() function. I've reworked it a million times and I can't figure out where it's going wrong. It compiles fine but gives me a segmentation fault.

Here's the program:

#include <stdio.h>
#include <stdlib.h>

struct node
{
  char *card;
  struct node *next;
};
typedef struct node node_t;

//print
void printlist(node_t *head)
{
  node_t *temp = head;

  while (temp != NULL)
  {
    printf("%s\n", temp->card);
    temp = temp->next;
  }
}



node_t *create_new_node(char *card)
{
  node_t *result = malloc(sizeof(node_t));
  result->card = card;
  result->next = NULL;

  return result;
}

node_t *insert_at_head(node_t **head, node_t *node_to_insert)
{
  // have node_to_insert point to the head
  node_to_insert->next = *head;
  // now have the head point to the node_to_insert
  *head = node_to_insert; // having this pointer requires the **head parameter
  return node_to_insert;
}

// add new node at the end of the list
void add_node(node_t *head, node_t *new_node)
{
    node_t *tmp = head;

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



int main(void)
{
  char *card_list[5] = {"counterspell", "black lotus", "giant growth", "mountain", "forest"};
  int len = sizeof(card_list)/sizeof(card_list[0]);

  node_t *head = NULL;
  node_t *temporary;


  for (int i = 0; i < len; i++)
  {
    temporary = create_new_node(card_list[i]);
    if ( i == 0)
    {
      head = insert_at_head(&head, temporary);
    }
    else
    {
      add_node(head, temporary);
    }
  }

  printlist(head);

  return 0;
}

Upvotes: 0

Views: 39

Answers (1)

Vlad from Moscow
Vlad from Moscow

Reputation: 310980

The used approach by you is incorrect.

You should append data to the list not pointers to nodes.

The functions can be defined the following way.

node_t * create_new_node( char *card )
{
    node_t *result = malloc( sizeof( node_t ) );

    if ( result != NULL )
    {
        result->card = card;
        result->next = NULL;
    }

    return result;
}

These two functions will be simpler and less error-prone if to pass the pointer to the head node by reference.

int insert_at_head( node_t **head, char *card )
{
    node_t *node_to_insert = create_new_node( card );
    int success = node_to_insert != NULL;

    if ( success )
    {
        node_to_insert->next = *head;
        *head = node_to_insert;
    }

    return success;
}

// add new node at the end of the list
int add_node( node_t **head, char *card )
{
    while ( *head != NULL )
    {
        head = &( *head )->next;
    }

    *head = create_new_node( card );

    return *head != NULL;
}

And in main you can write

char *card_list[] = 
{
    "counterspell", "black lotus", "giant growth", "mountain", "forest"
};
size_t len = sizeof( card_list ) / sizeof( card_list[0] );

node_t *head = NULL;

for ( size_t i = 0; i < len; i++ )
{
    add_node( &head, card_list[i] );
}

Pay attention to that in general you should make a copy of the passed string in each node.

Upvotes: 1

Related Questions