Linklist insertion of node displays nothing

Question

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

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

void insert( struct node *q,int num)
{
    struct node *temp;

    if( q == NULL)
    {
        q = (struct node*)malloc(sizeof(struct node));
        q->data = num;
        q->next = NULL;
    }
    else
    {
        temp = q;
        while( temp != NULL)
        {
            temp = temp->next;
        }
        temp = (struct node*)malloc(sizeof(struct node));
        temp->data = num;
        temp->next = NULL;
    }
}

void display(struct node *q)
{
    struct node *temp;
    temp = q;
    while(temp != NULL)
    {
        printf("%d",temp->data);
        temp = temp->next;
    }
}



int main()
{
    struct node *a;
    a = NULL;
    insert( a,13);
    insert( a,13);
    display(a);
    return 0;
}

In the insert function q is a pointer to struct node which is initialized to NULL.

Here I am seeing 1st if q is NULL or not. If it is null then I am allocating heap memory, data and next pointer, in this way q is now a pointer which is dereferencing to 1st data. If q is not NULL, then I take a temp pointer which points to a struct node which is being pointed by q, so till temp becomes NULL temp goes to temp->next, then it allocates heap memory, puts data and next pointer to NULL.

But it is showing nothing for my display function please correct me on this, and on how stack and heap memory is used in linked list.

Answer 1

In two ways you can solve this problem.

1. Pass the address of the q in the function.
2. change the return type from void to node. return root node each time after inserting the new node (this method is easy but not advisable).

Answer 2

Replace this code from yours:

    while( temp != NULL)
    {
        temp = temp->next;
    }
    temp = (struct node*)malloc(sizeof(struct node));
    temp->data = num;
    temp->next = NULL;

by

    while( temp->next != NULL)
    {
        temp = temp->next;
    }
    temp->next = (struct node*)malloc(sizeof(struct node));
    temp->next->data = num;
    temp->next->next = NULL;

Answer 3

Recall that in C arguments are pass-by-value, including pointer arguments.

When q == NULL, you're allocating memory and assigning that memory to q, but this won't change q outside your function: only the copy of q inside your function will be changed.

In order to change what the argument q points to, and have those changes reflected outside your function, you'll need to pass a pointer to a pointer, e.g.:

void insert(struct node **q, int num)

And change how you're using q, e.g.

if (*q == NULL)
    *q = (struct node *) malloc(sizeof(struct node));

Furthermore, in your else case, you should loop until temp->next == NULL, then add your new node with:

temp->next = (struct node*) malloc(sizeof(struct node));

Answer 4

You need to return the pointer allocated in insert.

In main, a is a pointer to NULL. After the first insert, a is still a pointer to NULL because q has the pointer, not a.

The value of a is the address you can find a struct node. q is a copy of the value of a, so is NULL. When you malloc() it assigns a value to q which is the address of a struct node but it does not change a!

Either:

/* a has a value and it doesn't malloc q */
main() {
   struct node a = {0};

   insert(&a, 13);
}

or

/* you return the value of q (address of struct node) and assign it to a */
struct node *insert(struct node *q, int num) {
   blah blah

   return q;
}

main() {
   struct node *a = NULL;

   a = insert(a, 13);
}

or

/* I'm finding this hard to explain because of 2 redirections */
void insert( struct node **q, int num ) {

   if ( *q == NULL ) {
   *q = malloc() etc etc
   }
}

main() {
   struct node *a = NULL;

   insert(&a, 13);
}

But you also make a similar mistake in the second half of insert. You need to allocate the memory and assign it to next, not the other way around.

Answer 5

It looks like the problem is that you iterate off the end of your list. So when temp becomes null you say, hey, I have found the end. And you make a new node. But you never point the previous node at your new node.

I would change this:

while( temp != NULL)

To this:

while( temp->next != NULL)

So you will still have a reference to your list when you reach the end. You will then need to change the rest of your logic accordingly, but at least it will be possible.

And as others are saying, your function also does not work for the initial node. I would consider having a separate function for initializing an empty list. But that is a style choice mainly.

Answer 6

change

insert( struct node *q,int num) toinsert( struct node **q,int num)

and inside main(), change

insert( a,13) to insert( &a,13)

You need to modify actual argument not the formal argument, so use pass by address not pass by value,
means, inside insert() when you are assing value to q it is not being reflected to a, as you are just passing the a's value, in order to make the change reflected to a pass a's address.

also,
one more problem is inside the else block in insert()
change while( temp != NULL) to while( temp->next != NULL)

Answer 7

Here's a fixed version of your program. Problem is that pointers are copied into functions by value, so when your function exits, the pointer passed in a in this case, is not allocated to anything. The only thing you'd done is leaked memory, by allocating some and not free'ing it.

Solution is to pass the pointer by reference, and in C that is done with a pointer-to-pointer.

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

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

void insert( struct node **q,int num)
{
    struct node *temp;

    if( *q == NULL)
    {
        *q = (struct node*)malloc(sizeof(struct node));
        (*q)->data = num;
        (*q)->next = NULL;
    }
    else
    {
        temp = *q;
        while( temp != NULL)
        {
            temp = temp->next;
        }
        temp = (struct node*)malloc(sizeof(struct node));
        temp->data = num;
        temp->next = NULL;
    }
}

void display( struct node *q)
{
    struct node *temp;
    temp = q;
    while(temp != NULL)
    {

        printf("%d",temp->data);
        temp = temp->next;
    }
}



int main()
{
    struct node *a;
    a = NULL;
    insert( &a,13);
    insert( &a,13);
    display(a);
    free(a->next); //de-alloc memory
    free(a);
    return 0;
}

Answer 8

You must use pointer to pointer at the insert() function, because you allocate new memory with malloc(), but the pointer will still point to NULL. So to modify the pointer itself you must use a pointer to pointer if you do it with parameters. IMHO, it will be much better if you return the pointer.