views:

151

answers:

5

I want to print the reverse of a linked list. I am doing it by recursion. But while calling read(temp) in function read, it gives a BUS ERROR.

Any reasons why is this happening ??

#include <iostream>

using namespace std;

struct node
{
    int info;
    node *next;
};

void read(node *start)
{
    node *temp = start->next;

    if(start == NULL)
        cout<<start->info<<"\n";
    else
    {
        read(temp);
        cout<<start->info<<"\n";
    }
}

int main()
{
    node *start = NULL;

    for(int i=0;i<5;i++)
    {
        node *temp = new node;
        temp->info=i;
        temp->next=NULL;

        if(start == NULL)
            start = temp;
        else
        {
            temp->next = start;
            start = temp;
        }
    }
    read(start);    
}
+4  A: 

This looks to be the culprit:

if(start == NULL)
    cout<<start->info<<"\n";

If start is NULL, you cannot dereference it.

Looking closer, the root of the problem is:

node *temp = start->next;

You are doing this before checking if start is NULL.

Finally, I find it odd that you are using the name 'read' for a function that is printing the data.

R Samuel Klatchko
+1  A: 

In function read, when start == NULL, you cannot dereference it, as you do with start->info.

Didier Trosset
+1  A: 

The line

if(start == NULL)
        cout<<start->info<<"\n";

makes no sense at all. How can you get start->info when start is NULL? Also, simply fixing this line will not fix your code, there is the same error in other places.

This looks like homework, so I am not posting the fixed version.

MAK
+1  A: 

You're waiting until start is NULL before doing the first de-reference (which will dump core). You need to run forward until the successor to start is NULL:

Replace the line:

if (start == NULL)

with:

if (start->next == NULL)

in read() and you'll get:

0
1
2
3
4

which is what I think you were after.

But you'll also want to guard against an empty list, so the full implementation should be something like the following. I'm not a big fan of polluting namespaces so I'd probably drop the using and explicitly qualify cout and endl (which I prefer to "\n") as well.

In addition, there's no actual need for temp if you want to save a few lines. Compilers are more than smart enough to cache things like start->next without you doing it manually.

void read (node *start) {
    if (start != 0) {
        if (start->next == 0) {
            std::cout << start->info << std::endl;
        } else {
            read (start->next);
            std::cout << start->info << std::endl;
        }
    }
}
paxdiablo
A: 

thanks a lot :D ... plenty of silly mistakes committed.

Ankit