Inserting a node at the end of a doubly linked list

Question

I tried inserting node at the end of doubly-linked list with a insert function but the process execution stops abruptly after second insertion. I tried checking out address of all pointer variable used in insert function and they show that my code works fine until the third insert function. How can I fix my code?

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

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

void insert(struct node** header,int data) {
    struct node* newnode=(struct node*)malloc(sizeof(struct node*));
    newnode->data=data;
    newnode->prev=NULL;
    newnode->next=NULL;

    if(*header == NULL) {
        *header=newnode;
        return;
    }

    struct node* temp = *header;

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

    temp->next = newnode;
    newnode->prev = temp;
    printf("%d\n**\n", temp->next);
}

void main() {
    struct node* head = NULL;
    insert(&head, 4);
    insert(&head, 45);
    printf("\n main head %d", head);
    insert(&head, 8);
    printf("testing");
    insert(&head, 69);
}

Suggestion regarding architecture: have `insert` take a `struct node *` and return a `struct node *`; I've found this a bit easier to understand, since you don't have to screw around with double pointers. — APerson, Nov 06 '14 at 14:06
You should either use the `%p` format to print pointers or keep the `%d` format and print the node data. — M Oehm, Nov 06 '14 at 14:09
@APerson: Or maybe create a list struct that keeps both head and tail and have the functions work on pointers to that struct. (Returning the new head always creates a redundancy - `head = something(head)` -, which I find uglier than passing double pointers.) — M Oehm, Nov 06 '14 at 14:13

Jabberwocky · Accepted Answer · 2014-11-06T14:14:59.080

2

You are not allocating enough memory

struct node* newnode=(struct node*)malloc(sizeof(struct node*)); 
// here you allocate only 4 bytes (or 8 bytes on a 64 bit system)

should be:

struct node* newnode = (struct node*)malloc(sizeof(struct node));

Then everything works fine.

edited Nov 06 '14 at 14:14

answered Nov 06 '14 at 14:09

Jabberwocky

48,281
17
65
115

2

or `sizeof *newnode`, leaving no doubt that which is about to be allocated matches the pointer type to which it is assigned. [And lose the cast on `malloc()`](http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc) – WhozCraig Nov 06 '14 at 14:11

Inserting a node at the end of a doubly linked list

1 Answers1