CODEWITHSUNDEEP
clinked-list
Dark
Dark

Reputation: 333

Adding item to the end of linked list

I am trying to add an item to the end of a linked list, or simply add an item if the linked list is empty.

So far I have

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

newnode->data=item;
newnode->next=NULL;
if (curr == NULL){
    curr=newnode;
}
else {
    while(curr->next == NULL){
        curr=curr->next;
    }
    curr->next=newnode;
}
return 1;

So how does the code look? Any help figuring out what is wrong would be greatly appreciated.

Upvotes: 0

Views: 1635

Answers (6)

AnT stands with Russia
AnT stands with Russia

Reputation: 320381

It might be a good opportunity to learn the double-pointer idiom, which is rather useful for performing scanning modifications of linked lists. In your case that would be

struct node *newnode, **pnext;

newnode = malloc(sizeof *newnode);
newnode->data = item;
newnode->next = NULL;

for (pnext = &head; *pnext != NULL; pnext = &(*pnext)->next);

*pnext = newnode;

Note that in this approach you no longer need any branching. It works uniformly regardless of whether you are adding the new element at the beginning of the list, in the middle or at the end.

Upvotes: 0

Nobilis
Nobilis

Reputation: 7448

This is not going to go anywhere, you start at the head and if that's not empty then you say "keep going while next is empty". If next is not empty you'll still stay at the head.

To add at the end of the list you'll need something like:

else
{
    while(curr->next != NULL) // keep going while curr->next is not null
    {
        curr = curr->next; // advance the pointer
    }
    // Here curr->next will definitely be null
    curr->next = newnode; // now point curr->next to the new node
}

Someone already pointed out that in the case of curr == NULL in the first check, once you do the assignment you don't return the pointer and head never gets initialised.

You can get away with that either by having head declared outside the scope of this function, or have a pointer to a pointer in your function signature, e.g.:

int add_node(struct node** head) // I'll assume this is how you've declared your add function
                     // ^^  notice the pointer to pointer
{
    struct node* curr = *head; // curr assignment changes slightly
    struct node* newnode = malloc(sizeof(struct node));
    newnode->data = item;
    newnode->next = NULL;
    if (curr == NULL)
    {
        *head = newnode; // this will dereference the pointer to the pointer and update head without returning anything
    }

The rest stays the same.

Upvotes: 0

user1952500
user1952500

Reputation: 6771

Some changes needed:

  1. You need to update head also in the case where you check curr == NULL

  2. The while loop should be while (curr->next != NULL)

Upvotes: 0

Hogan
Hogan

Reputation: 70513

This won't work

if (curr == NULL){
   curr=newnode;}

You need this:

if (curr == NULL){
   head=newnode;}

The way you had it curr is a local variable pointing to the new element and goes away with the function return. You never keep track of a new one.

And as others have said you need != in the loop.

Upvotes: 2

AliciaBytes
AliciaBytes

Reputation: 7429

There's one thing I can spot that looks weird

while(curr->next == NULL){
     curr=curr->next;}

Doubt you want to go to the next node while there isn't one. What you probably wanted was != in the condition instead of ==

Upvotes: 1

fghj
fghj

Reputation: 9394

while(curr->next == NULL)

should be

while(curr->next != NULL)

Upvotes: 0

Related Questions