views:

201

answers:

6

I am getting segmentation fault in this code but i cant figure out why. I know a segmentation fault happens when a pointer is NULL, or when it points to a random memory address.

 q = p;
        while(q -> link != NULL){
            q = q -> link;
        }
        t = new data;
        t -> city = cityName;
        t -> latitude = lat;
        t -> longitude = lon;
        q -> link = t;

This is the error am actually getting in console

Terminal Output

Or written out: line 33: 2219 Segmentation fault sh "${SHFILE}"

+7  A: 

In the else clause in Database::add, you do not set t->link = NULL, so it is uninitialized.

You should add a constructor for data that initializes its members, or use the value-initializing new to ensure that everything is initialized correctly:

t = new data(); // note the parentheses
James McNellis
+3  A: 

It's possible that it's because when you're adding a new node to the end of the list:

else{
    q = p;
    while(q -> link != NULL){
        q = q -> link;
    }
    t = new data;
    t -> city = cityName;
    t -> latitude = lat;
    t -> longitude = lon;
    q -> link = t;
}

you're not setting t->link = NULL;.

Michael Burr
+2  A: 

Compile with -g as an argument to g++

Then from command line "gdb (binary name)"

inside gdb, "run" and it will execute until the fault

type "bt" to see a stack trace

Mike
sorry for not spoonfeeding you what you needed. i'll remember not to answer your questions in the future
Mike
What tells you he's the one who downvoted you? (No, it wasn't me. ;))
MetalMikester
I downvoted because A. The user appears to be a beginner and probably is not aware of a "stack trace". B. The user is not running from a commandline and probably has no experience with gdb. C. Nothing says the user has GDB installed. D. You spent no time looking at the code and answering the question, you simply put in a response that is canned, etc.
Billy ONeal
I still think giving OP a tip to use a debugger is more valuable than pointing out the flaw itself. Teach them to fish, etc. Figuring out how to use gdb is an exercise to the reader
Mike
Maybe that's what you think, but I'm looking at the question and it says, "What's wrong with this code". Your answer does not answer that question, and you assume the user knows too much given what he's posted. For example, the user is using netbeans. You don't even know he knows what g++ is much less gdb. Therefore I downvoted.
Billy ONeal
as a sidenote, this is clearly homework so i don't feel bad giving generic advice.
Mike
If you want to give generic advice, that's fine, but generic advice belongs as an aside to an actual answer to the OP's specific question, or in a comment.
Billy ONeal
+1: Debuggers are especially useful when dealing with memory-related issues -> good answer.
gorsky
+3  A: 

You are not setting Database::data::link to NULL in Database::add:

    t = new data;
    t -> city = cityName;
    t -> latitude = lat;
    t -> longitude = lon;
    t -> link = NULL;

EDIT: I would add a constructor for Database::data to initialize the various members. Something like:

class Database {
    struct data {
        data(): latitude(0.0), longitude(0.0), link(NULL) {}
        ...
    };
    ...
 };

Then you do not have uninitialized memory to worry about.

D.Shawley
+1  A: 

There may be other issues but here's one:

else{
    q = p;
    while(q -> link != NULL){
        q = q -> link;
    }
    t = new data;
    t -> city = cityName;
    t -> latitude = lat;
    t -> longitude = lon;
    q -> link = t;
}

You never set t->link to null and therefore it's filled with junk.

Billy ONeal
+1  A: 

You need to assign t's link to NULL:

else{
    q = p;
    while(q -> link != NULL){
        q = q -> link;
    }
    t = new data;
    t -> city = cityName;
    t -> latitude = lat;
    t -> longitude = lon;
    t -> link = NULL; // add this
    q -> link = t;
}
Jeremy Bell
Heh, we all answered at the same time.
Jeremy Bell