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.
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.
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 ==
while(curr->next == NULL)
should be
while(curr->next != NULL)
Some changes needed:
You need to update head also in the case where you check curr ==
NULL
The while loop should be while (curr->next != NULL)
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.
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.
