Why is my program terminating abruptly?

Question

I have a program for creating a singly linked list,adding a node at the beginning and end of the list. The program is producing the intended output,but it is crashing after producing the last output. I have created the linked list in main(),i have 2 functions for adding a node at the beginning and at the end respectively.

#include<stdio.h>
struct book
{

  char bname[20];
  char aname[20];
  int pages;
  struct book *next;
};


struct book *first;
struct book *current;
struct book *previous;
int main()
{
  int count=0,temp=0;


  for(int i=1;i<4;i++)
  {

    current=(struct book*)malloc(sizeof(struct book));//memory assigned to only the current structure
    if(current==NULL)
    {
      break;
    }
    if(first==NULL)
    {
      first=current; //if the first node value is null then it hadn't yet been processed,so current node is now the first;. 
    }
    if(previous!=NULL)
    {
      previous->next=current;//stores the current structure address to the next member(pointer) of the previous structure address
    }

    printf("\nBook Name:: ");
    scanf("%s",current->bname);

    printf("\nAuthor Name:: ");
    scanf("%s",current->aname);

    printf("\nPages::");
    scanf("%d",&current->pages);

    current->next=NULL;//if this is the last node
    //will again be filled up if there is a next structure

    previous=current;//the current node is the previous node for the next iteration 
  }


  current=first;
  while(current!=NULL)
  {
    count++;
    printf("\nBook Name:: %s Author Name:: %s Pages:: %d",current->bname,current->aname,current->pages);
    previous=current;//store to previous only because of freeing the current structure
    current=current->next;//next structure address of the current node is the current node of the next iteration.Now if this next address doesn't exist,then the current pointer does not exist and the loop terminates


  }
  addnodebeginning(first);

}


/*****************************inserting at the BEGINNING*********************************/

void addnodebeginning(struct book *first)
{
  int count=0,temp=0;
  current=(struct book*)malloc(sizeof(struct book));
  current->next=first;
  first=current;
  printf("\nBook Name:: ");
  scanf("%s",current->bname);

  printf("\nAuthor Name:: ");
  scanf("%s",current->aname);

  printf("\nPages::");
  scanf("%d",&current->pages);


  current=first;
  while(current!=NULL)
  {
    count++;
    printf("\nBook Name:: %s Author Name:: %s Pages:: %d",current->bname,current->aname,current->pages);
    previous=current;//store to previous only because of freeing the current structure
    current=current->next;//next structure address of the current node is the current node of the next iteration.Now if this next address doesn't exist,then the current pointer does not exist and the loop terminates


  }

  addnode_end(first);

}
/*****************************inserting at the end*********************************/

void addnode_end(struct node *first)
{

  current=first;

  while(1)
  {

    if(current->next==NULL)
    {
      struct book *newnode=(struct book*)malloc(sizeof(struct book));

      printf("\nBook Name:: ");
      scanf("%s",newnode->bname);

      printf("\nAuthor Name:: ");
      scanf("%s",newnode->aname);

      printf("\nPages::");
      scanf("%d",&newnode->pages);

      current->next=newnode;
      //newnode->next=NULL;
      break;
    }
    current=current->next;
  }

  current=first;
  while(current!=NULL)
  {

    printf("\nBook Name:: %s Author Name:: %s Pages:: %d",current->bname,current->aname,current->pages);
    previous=current;//store to previous only because of freeing the current structure
    current=current->next;
    free(previous);

  }

}

Why is this happening?

Answer 1

First of all,take care of the warnings. They are there for a reason. Secondly, you just include this at the end of the if test-

newnode->next=NULL;
 break;

Your program will then have no runtime crash.

Answer 2

In the addnodebeginning function, the first argument is passed by value, that means the value is copied and you modify the copy in the function. If you want to modify the passed value, you have to pass it by reference.

This is done by passing a pointer to the value, in this case a pointer to the pointer.

Answer 3

The code misses to #include <stdlib.h>. Which at least for calling malloc() on 64bit systems most probably is fatal.

If on a 64bit system and the prototype for malloc() is missing the compiler assumes int as the return value for malloc(). int most certainly is 32bit, where as the address malloc() tries to return is 64bit wide, so most likely the address value to be returned gets half of it cut off on return.

---

Also this line

void addnode_end(struct node *first)

should be

void addnode_end(struct book *first)

---

Update:

Also the code misses to initialise the member of a freshly malloc()ed node. Typically its next member shall be set to NULL explicitly.