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?

[Don't cast the result of `malloc` et al](http://stackoverflow.com/a/605858/440558). — Some programmer dude, Dec 30 '13 at 18:58
yes,I can assure u that it compiles perfectly fine! Have tested it more than once. — user3130420, Dec 30 '13 at 18:58
@Digital_Reality It compiles in c99 mode on gcc, but with 17 warnings (some serious) — , Dec 30 '13 at 18:59
@Digital_Reality `node` only spawns an warning that the struct is declared in the parameter, seems to be legitimate — , Dec 30 '13 at 19:04

score 2 · Answer 1 · answered Dec 30 '13 at 19:01

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.

answered Dec 30 '13 at 19:01

Some programmer dude

400,186
35
402
621

Can you show me the exact code for that.I am a bit new to pointers and the above program was entirely based on my understandings.I would love to do it the way you said. Help me out on this. – user3130420 Dec 30 '13 at 19:27

alk · Answer 2 · 2013-12-30T19:36:08.463

1

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.

edited Dec 30 '13 at 19:36

answered Dec 30 '13 at 18:57

alk

69,737
10
105
255

1

-1, this would cause a compile error, not a runtime error/crash. – TypeIA Dec 30 '13 at 19:01
1

@dvnrrs: No, this depends on compiler and it's settings. My gcc just swallowed all the sources above, without any error. Yes warnigns it issued a lot. Please recheck what you state. – alk Dec 30 '13 at 19:03
It didn't cause any compilation errors.But I have changed it though.Still the same problem. – user3130420 Dec 30 '13 at 19:07
1

@user3130420: You might like to compile with symbols (`-g` for gcc) and run the code using gdb. When it crashed than issue a `bt` command and see the stack-trace showing you were it crashed and how it got there. Also compile with all warning on (`-Wall -Wextra -pedantic`) and fix the code until no more warnings occur. – alk Dec 30 '13 at 19:13
@alk: I have corrected these two errors. The problem still persists.What to do sir? – user3130420 Dec 30 '13 at 19:25
@Please see the update to my answer. And although not the issue now you should correct what *Joachim Pileborg* mentions in his answer, as I might become an issue. – alk Dec 30 '13 at 19:38

score 1 · Accepted Answer · answered Dec 30 '13 at 19:52

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.

answered Dec 30 '13 at 19:52

arindrajit

76
1
7

Thanks a lot..Would love to vote you up,bt my reputations are not enough. – user3130420 Dec 30 '13 at 19:57

Why is my program terminating abruptly?

3 Answers3