Pointer resets on 2nd for loop?

[SillyNoob]

I'm at an absolute complete loss with this. I'm a using a pointer-pointer to "remember" the last node of a linked list so a function in the loop can add another node and then return the address of that new node so it's known for the next loop.

Sounds easy, right? Well i've found a way to mess it up :(.

The real program is spread across multiple files and i'd hate to go overkill with 500+ lines so i've cut it all down to what I hope are the bare essentials required for this problem to be fixed. It's hard sifting through lots of code and working out what will and won't be required to form the mental image required.

Out of all the mess below the part I want to draw your attention to is the for() loop in the Load() function right at the bottom.

class ScoreEntry
{
protected:
    ScoreEntry *Nextnode;
public:
    int Score;
    string Name;
    int Kills;
    ScoreEntry();
    ScoreEntry** Load(ifstream& infile, ScoreEntry **currEntry);
    ScoreEntry* GetNext();
    void SetNext(ScoreEntry *next);
};
ScoreEntry::ScoreEntry()
{
    Name = "";
    Score = 0;
    Kills = 0;
    Nextnode = 0;
}
ScoreEntry** ScoreEntry::Load(ifstream& infile, ScoreEntry **currEntry)
{
    // new node to store the data in
    ScoreEntry* newEntry = new ScoreEntry;
    // read in data to the new node
    infile >> newEntry->Name;
    infile >> newEntry->Kills;
    infile >> newEntry->Score;
    // end of file doesn't constitute a read error
    if(!infile.good() && !infile.eof()){
        delete newEntry;
        return 0;
    }
    if(*currEntry == 0){
        // the head is the first node
        *currEntry = newEntry;
    }else{
        // attach the node to end of the list
        (*currEntry)->SetNext(newEntry);
    }
    ScoreEntry** retEntry = &newEntry;
    return retEntry;
}
ScoreEntry* ScoreEntry::GetNext()
{
    return Nextnode;
}
void ScoreEntry::SetNext(ScoreEntry *next)
{
    Nextnode = next;
}

class HighScoreTable
{
public:
    HighScoreTable();
    int Load();
    ScoreEntry *ListHead;
};
HighScoreTable::HighScoreTable()
{
    ListHead = 0;
}
int HighScoreTable::Load()
{
    int entries = 0;
    ScoreEntry **currEntry = &ListHead;
    for(int i=0; highScrFile.good() && i<MAXENTRIES; i++){
        currEntry = (*currEntry)->Load(highScrFile, currEntry);
        if(*currEntry != 0){
            entries++;
        }
    }
    return entries;
}

The pointer-pointer currEntry is declared outside of the for() loop, and so should be in-scope until the function returns. The pointer being pointed to however (for some unknown reason) resets after the 1st loop!

ScoreEntry **currEntry = &ListHead;
for(int i=0; highScrFile.good() && i<MAXENTRIES; i++){ // <<<< *currEntry resets after stepping past here! :(
    currEntry = (*currEntry)->Load(highScrFile, currEntry);
    if(*currEntry != 0){
        entries++;
    }
}

After

currEntry = (*currEntry)->Load(highScrFile, currEntry);

Has executed *currEntry will show correctly as:

0x055c5fa8 {Score=data Name=data Kills=data ...} ScoreEntry *

but as soon I step past the for() line (as the 2nd iteration begins) *currEntry instantly transforms into:

*currEntry 0xcccccccc {Score=??? Name={...} Kills=??? ...} ScoreEntry *

What the ... ? It's acting as though it's suddenly come out of scope and had to be re-initialised. This completely defeats the purpose of the pointer-pointer as it's supposed to remember the last node of the linked list for the ScoreEntry Load() function.

Whhhyyyyyyyyyyyy !? :(

[histrungalot]

Not at home to check it out but I pretty sure that your problem is, you are returning a pointer to something on the stack.

    ScoreEntry** retEntry = &newEntry;
    return retEntry;

newEntry is a local variable and when you leave the Load function its address is no longer valid.

Why not use just a single pointer?

[mike_2000_17]

Calm down. There is always a good explanation for everything.

Your problem is with the lines 41 and 42. You are returning a pointer to a local variable. That variable happens to be a pointer as well, but that doesn't change anything. Your function returns a pointer to a local pointer variable. That pointer variable quickly goes out-of-scope when the next iteration of the for-loop begins. I suggest you do this instead:

f(*currEntry == 0){
  // the head is the first node
  *currEntry = newEntry;
  return currEntry;
}else{
  // attach the node to end of the list
  (*currEntry)->SetNext(newEntry);
  return &((*currEntry)->Nextnode);
}

That should fix your problem.

[SillyNoob]

Apologies for taking so long to reply. I tried to reply yesterday but Daniweb's anti-spam bot human test question kept refusing to accept my answers. Anyone else ever had this issue? Very strange indeed, maybe i'm an undercover bot for Skynet or something?

Anways, that'll be enough drivel because I did exactly as you said "mike_2000_17" and got it working! It freaking works...finally after two whole days of pain, pondering and trouble getting to sleep it finally works. It builds my linked list! :) Thank you.

ScoreEntry** ScoreEntry::Load(ifstream& infile, ScoreEntry** currEntry)
{
    // new node to store the data in
    ScoreEntry* newEntry = new ScoreEntry;
    // read in data to the new node
    infile >> newEntry->Name;
    infile >> newEntry->Kills;
    infile >> newEntry->Score;
    // end of file doesn't constitute a read error
    if(!infile.good() && !infile.eof()){
        delete newEntry;
        return 0;
    }
    if(*currEntry == 0){
        // the head is the first node
        *currEntry = newEntry;
        return &(*currEntry);
    }else{
        // attach the node to end of the list
        (*currEntry)->SetNext(newEntry);
        return &((*currEntry)->Nextnode);
    }
}

Just a couple of things though before I wrap this thread up.

1) But... soooo... it was the pointer local variable retEntry that was going out of scope and thus caused the pointer being pointed to in currEntry (*currEntry) to suddenly vanish into cyberspace. That's all well and good, a local variable going out of scope when the function returns. Nothing new to me there. BUT why did it still remain "alive" if you would until the next iteration of the loop? Would it not have dissappeared as soon as the function returned?

2) I couldn't figure out a way to use the GetNext() function to grab ->Nextnode. At the moment everything in all of the classes is public just because I had trouble using functions to access even protected data. This of course is kind of stupid because you shouldn't be able to directly access node data from main() or any other non-member functions.

So instead of

return &((*currEntry)->Nextnode);

I wanted to try something like:

currEntry = &((*currEntry)->GetNext());

with the GetNext() function looking like:

ScoreEntry* ScoreEntry::GetNext()
{
    return Nextnode;
}

Of course the rubbish code I write throws up all kind of errors. This time it's back to the popular with:

currEntry = &((*currEntry)->GetNext());

giving

Error 1 error C2102: '&' requires l-value