C linked list inserting node at the end

Question

I'm having some trouble with my insertion method for a linked list in C. It seems to only add at the beginning of the list. Any other insertion I make fail. And this CodeBlocks debugger is so hard to understand I still don't get it. It never gives me value, just addresses in memory. Anyway this is my function. Do you see any reason why it's failing?

/* function to add a new node at the end of the list */
int addNodeBottom(int val, node *head){

    //create new node
    node *newNode = (node*)malloc(sizeof(node));

    if(newNode == NULL){
        fprintf(stderr, "Unable to allocate memory for new node\n");
        exit(-1);
    }

    newNode->value = val;

    //check for first insertion
    if(head->next == NULL){
        head->next = newNode;
        printf("added at beginning\n");
    }

    else
    {
        //else loop through the list and find the last
        //node, insert next to it
        node *current = head;
        while(current->next != NULL)
        {
            if(current->next == NULL)
            {
                current->next = newNode;
                printf("added later\n");
            }
            current = current->next;
        }
    }
    return 0;
}

Then in main, only 929 is added.

   //testing addNodeBottom function
    addNodeBottom(929, head);
    addNodeBottom(98, head);
    addNodeBottom(122, head);
    addNodeBottom(11, head);
    addNodeBottom(1034, head);

Answer 1

I know this is an old post but just for reference. Here is how to append without the special case check for an empty list, although at the expense of more complex looking code.

void Append(List * l, Node * n)
{
    Node ** next = &list->Head;
    while (*next != NULL) next = &(*next)->Next;
    *next = n;
    n->Next = NULL;
}

Answer 2

This works fine:

struct node *addNode(node *head, int value) {
    node *newNode = (node *) malloc(sizeof(node));
    newNode->value = value;
    newNode->next = NULL;

    if (head == NULL) {
        // Add at the beginning
        head = newNode;
    } else {
        node *current = head;

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

        // Add at the end
        current->next = newNode;
    }

    return head;
}

Example usage:

struct node *head = NULL;

for (int currentIndex = 1; currentIndex < 10; currentIndex++) {
    head = addNode(head, currentIndex);
}

Answer 3

The new node is always added after the last node of the given Linked List. For example if the given Linked List is 5->10->15->20->25 and we add an item 30 at the end, then the Linked List becomes 5->10->15->20->25->30. Since a Linked List is typically represented by the head of it, we have to traverse the list till end and then change the next of last node to new node.

/* Given a reference (pointer to pointer) to the head
   of a list and an int, appends a new node at the end  */


    void append(struct node** head_ref, int new_data)
    {
    /* 1. allocate node */
         struct node* new_node = (struct node*) malloc(sizeof(struct node));

        struct node *last = *head_ref;  /* used in step 5*/

    /* 2. put in the data  */
        new_node->data  = new_data;

    /* 3. This new node is going to be the last node, so make next 
          of it as NULL*/
        new_node->next = NULL;

    /* 4. If the <a href="#">Linked List</a> is empty, then make the new node as head */
        if (*head_ref == NULL)
        {
       *head_ref = new_node;
       return;
        }  

    /* 5. Else traverse till the last node */
        while (last->next != NULL)
        last = last->next;

    /* 6. Change the next of last node */
        last->next = new_node;
        return;    
}

Answer 4

I would like to mention the key before writing the code for your consideration.

//Key

temp= address of new node allocated by malloc function (member od alloc.h library in C )

prev= address of last node of existing link list.

next = contains address of next node

struct node {
    int data;
    struct node *next;
} *head;

void addnode_end(int a) {
    struct node *temp, *prev;
    temp = (struct node*) malloc(sizeof(node));
    if (temp == NULL) {
        cout << "Not enough memory";
    } else {
        node->data = a;
        node->next = NULL;
        prev = head;

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

        prev->next = temp;
    }
}

Answer 5

After you malloc a node make sure to set node->next = NULL.

int addNodeBottom(int val, node *head)
{    
    node *current = head;
    node *newNode = (node *) malloc(sizeof(node));
    if (newNode == NULL) {
        printf("malloc failed\n");
        exit(-1);
    }    

    newNode->value = val;
    newNode->next = NULL;

    while (current->next) {
        current = current->next;
    }    
    current->next = newNode;
    return 0;
}    

I should point out that with this version the head is still used as a dummy, not used for storing a value. This lets you represent an empty list by having just a head node.

Answer 6

This code will work. The answer from samplebias is almost correct, but you need a third change:

int addNodeBottom(int val, node *head){

    //create new node
    node *newNode = (node*)malloc(sizeof(node));

    if(newNode == NULL){
        fprintf(stderr, "Unable to allocate memory for new node\n");
        exit(-1);
    }

    newNode->value = val;
    newNode->next = NULL;  // Change 1

    //check for first insertion
    if(head->next == NULL){
        head->next = newNode;
        printf("added at beginning\n");
    }

    else
    {
        //else loop through the list and find the last
        //node, insert next to it
        node *current = head;
        while (true) { // Change 2
            if(current->next == NULL)
            {
                current->next = newNode;
                printf("added later\n");
                break; // Change 3
            }
            current = current->next;
        };
    }
    return 0;
}

Change 1: newNode->next must be set to NULL so we don't insert invalid pointers at the end of the list.

Change 2/3: The loop is changed to an endless loop that will be jumped out with break; when we found the last element. Note how while(current->next != NULL) contradicted if(current->next == NULL) before.

EDIT: Regarding the while loop, this way it is much better:

  node *current = head;
  while (current->next != NULL) {
    current = current->next;
  }
  current->next = newNode;
  printf("added later\n");