pointers within a loop, maybe an easy problem

Posted by Trom on Fri 09 May 2008 05:05 PM — 5 posts, 17,791 views.

Trom Fri 09 May 2008 05:05 PM #0

I've never learned c language from books or anything so this maybe a simple problem.


//QUESTLIST_DATA *quest_list; its global
QUESTLIST_DATA *ql;
word = fread_word( fp );
if ( !str_cmp(word,"QL") || !str_cmp(word,"QUESTLIST") ) {
    ql = new_ql();
    fread_ql ( ql, fp );
    if ( quest_list != NULL )
        ql->next = quest_list;
    quest_list = ql;
    free_ql(ql);
}
else if ( !str_cmp( word, "END"    ) )
    break;

What does the above code do?
----------------------------
The above is the code i have within a loop. The problem is somewhat obvious. Memory is allocated to 'ql' before its used and then its filled with values, then added to the main global 'quest_list'. 'ql' was supposed to be free'd after the values were passed to the global 'quest_list', but instead it clears part of the 'quest_list' becuase 'ql' still retains the reference.

What I have tried
-----------------
Now i didn't want to have it use new_ql over and over without free'ing it since that would probably be bad for memory use. I also tried before having it allocate memory to ql once (before the loop begins) and i've also tried not having a free_ql at all. (the end result was faulty, users would lockup the mud when they connect.. the quest_list produces infinite loops somehow).

To The Point
------------
What i was hoping to do is free_ql() but not losing the values, just killing the reference so 'ql' can be used when the loop reaches its beginning again. I'm unsure of how to do that. Any help would be appreciated :)

Nick Gammon Australia Forum Administrator Fri 09 May 2008 08:59 PM #1

This code doesn't work, right? It looks sort of strange.

For example, even if ql is NULL, you are freeing it.

What does fread_ql do? Does it also allocate the pointer?

Quote:

Now i didn't want to have it use new_ql over and over without free'ing it since that would probably be bad for memory use.

Without knowing what fread_ql does, it is perfectly normal to allocate memory for something like a list of quests and not free it. After all, they have to take memory somewhere.

Trom Sat 10 May 2008 05:10 AM #2

Sorry for not being clear. I followed the use of free_char new_char and fread_char.

new_ql() - returns an allocated new ql, ready for use.
fread_ql() - reads the values from the actual file and stores it in the ql specified within the argument.
free_ql() - places the ql from the arguments in a global deadspace list. new_ql checks the deadspace list before allocating another to see if any existing structure is available (thats already taking up memory but not in use).

I've made a quest editor and the quest is held within quest_list. I named the nodes within the quest_list, 'ql'. Theres a way with the quest editor to remove an entry by making the id 0. When new_ql is called, it first searches the global quest_list and finds an entry thats id == 0. Then returns that instead of allocating another (then it searches deadspace, then finally if both are not available, it creates a new allocation).

ql is basically a temp structure. I was hoping to just assign the node to the global quest_list, then get rid of ql since it would linger.

The problem remains though that when reading two entries from a file (two linklist nodes), the first one remains, no other.

Nick Gammon Australia Forum Administrator Sat 10 May 2008 05:40 AM #3

You allocate memory for the quest, place it in the linked list, and then free the same pointer. I am not surprised it is not still there.

Anyway, I think I would amend it like this:


if ( !str_cmp(word,"QL") || !str_cmp(word,"QUESTLIST") ) {
    ql = new_ql();
    fread_ql ( ql, fp );
    ql->next = quest_list;
    quest_list = ql;
}

You are obviously adding to the head of the list, so you need to add the existing list to ql->next (even if the existing list is NULL). Then the new item becomes the new list (head). I wouldn't free the quests, that would certainly crash something.

Trom Sat 10 May 2008 06:05 AM #4

Thank you again for all the help. It is now working fully :)