CODEWITHSUNDEEP
c++
Frustrated Coder
Frustrated Coder

Reputation: 4697

Error in a Link List

I wrote the following code for a Link list to create a Book its serial no. and search it. I am using linked list in it.
When I add my first entry , it is added successfully, but when I add second entry it shows segmentation fault. I am not able to figure out why. Please help.
Thanks in advance.
Code:

#include<iostream>
#include<string>
#include<fstream>
#include<cstdlib>
using namespace std;

struct book
{
  int accno;
  string name;
  book* next;
};



int main()

{
  bool flag=false;
  int x,m;
  string s;
  book* front=NULL;
  book* n;
  do
    {

      cout<<"\nPlease select the following:\n1.Create and append\n2.Search\n3.Exit";
      cin>>m;
      switch(m)
    {
    case 1:
      n=new book();
      cout<<"\nEnter the book name: ";
      cin>>s;
      cout<<"\nEnter the acc no.: ";
      cin>>x;

      if(front==NULL)
        {
          front=n;
        }
      else
        {   n=front;

          while(n->next!=NULL)
        {
          n=n->next;
        }
          n=n->next;
        }

      n->accno=x;
      n->name=s;
      break;


    case 2:
      cout<<"Enter the roll no.";
      int y;
      cin>>y;
      if(front==NULL){cout<<"Doesnot exist\n"; break;
      }

      else
        {
          n=front;
          while(n->accno!=y && n->next!=NULL)
        {
          n->next=n;
        }
          cout<<"Book name is:"<<n->name;
          cout<<"\nAccno is: "<<n->accno;

        }
      break;


    case 3: flag=true;
      break;


    }
    }

  while(flag==false);

  return 0;
}

Upvotes: 1

Views: 94

Answers (5)

P&#233;ter T&#246;r&#246;k
P&#233;ter T&#246;r&#246;k

Reputation: 116306

Here

while(n->next!=NULL)
{
  n=n->next;
}
n=n->next;

you iterate through the linked list to find the last element, then step past it. After this, n will be null.

What you are missing is creating a new element and appending it to the end of the list.

And here

n->accno=x;
n->name=s;

you must also assign n->next = null, otherwise your list won't be properly terminated.

Also, when searching for a book, here

while(n->accno!=y && n->next!=NULL)
{
  n->next=n;
}
cout<<"Book name is:"<<n->name;
cout<<"\nAccno is: "<<n->accno;

after exiting the loop, either you found the book or n is null. You must check which is the case before trying to dereference n, otherwise you will again get a segfault if the book you are looking for is not in the list.

Upvotes: 6

Jan Hudec
Jan Hudec

Reputation: 76376

  1. Learn to write a linked list (so if this is a homework targeted at learning them, it's valid, but it is not tagged as such), but never ever do it in practice. There is a standard library and there is boost and they have all data structures you'll need unless you do something really special.
  2. You have C++, so don't write C-style code. book should have a constructor that should initialize it's members. The list head should probably be encapsulated in the class too and manipulated using it's methods.
  3. You never set n->next, so don't be surprised it never contains anything meaningful.
  4. You re-use n in the loop forgetting the object you constructed (memory leak).
  5. Than you get the NULL at the end of the list instead of the object you constructed.

Upvotes: 4

Eric Z
Eric Z

Reputation: 14535

This is a buggy code:

  1. You need null the "next" field when you add a new node:

    case 1: new book(); n->next = NULL; ...

  2. You have the memory leakage

Upvotes: 1

vrajs5
vrajs5

Reputation: 4126

What are you doing here?

case 1:
    n=new book();
    cout<<"\nEnter the book name: ";
    cin>>s;
    cout<<"\nEnter the acc no.: ";
    cin>>x;
      if(front==NULL) 
     {
        front=n;
      } 
   else
      {   
         n=front;
    }
 while(n->next!=NULL)
      {
        n=n->next;
      } 
       n=n->next;
      }
      n->accno=x;
    n->name=s; 
   break;

You have created new book and assigned it to n, in first case its ok becasue your are directly assigning it to front. But in other case you should iterate list using someother variable (temp), when your write n = front, your have already lost your new book object pointer. Hope you got your answer.

Upvotes: 1

Tony The Lion
Tony The Lion

Reputation: 63310

here lies your problem: 

 ....
    else
    {   
        n=front;  

        while(n->next!=NULL)  //access to next will cause seg fault!!!
        {
              n=n->next;
        }
            n=n->next;  // step once more, now we have NULL on second add...
    }

also, where is n->next being assigned? I don't see it anywhere?

Upvotes: 1

Related Questions