On Tue, 5 Feb 2002, Justin Adler wrote:
> I know u have to free anything that is CREATE'd, which these
> structures are ... but i'm not sure how.
This isn't a good simplification. You need to deallocate (free) anything
which you allocate that you are no longer going to be using. Otherwise,
the memory sits there until (if you have at least a half-way decent OS)
your program ends. If you're on a non-NT-based Microsoft OS, the memory
may sit there until you reboot.
However, misunderstanding or oversimplifying this concept isn't what has
you in trouble here. We'll focus on the following snip of the code,
instead:
> while ( (rent = GET_RENT(ch)) != NULL) {
> if (rent->hours < 1) {
> REMOVE_FROM_LIST(rent, GET_RENT(ch), next);
> free(rent);
> } else
> rent->hours--;
>
> rent = rent->next;
> }
There are several errors in this code.
First, recall that the code for the while condition is executed at the
beginning of each iteration of the loop. Here's a step-by-step look at
one iteration through the loop:
0. Set rent = GET_RENT(ch).
1. If rent == NULL, break.
2. If rent->hours < 1, remove it from the list and free its memory.
Otherwise, decrement rent->hours.
3. Set rent = rent->next.
4. Goto step 0.
So what's rent equal to on the first iteration? GET_RENT(ch). How about
on the second iteration? GET_RENT(ch). Third? Same. Nth? Same. Note
that step 3 might as well not even be there because right after it, you
loop back to the condition, and rent is reset to GET_RENT(ch). What we
need to do is move the initialization before the loop:
rent = GET_RENT(ch); /* Initialize. */
while (rent != NULL) { /* Now loop on it. */
. . . /* If block elided for brevity. */
rent = rent->next;
}
Alas! all is not well with our loop, yet. If we consider an element in
the character's rent list with hours >= 1 (i.e., the if condition is
FALSE, so only the else clause and non-conditional code is executed), the
following is executed in the loop's body:
rent->hours--;
rent = rent->next;
That looks okay. Next, we consider an element with hours < 1 (the if
condition is TRUE):
REMOVE_FROM_LIST(rent, GET_RENT(ch), next);
free(rent);
rent = rent->next;
It's hopefully easy to see a problem in this code right away. We're
trying to read rent->next after we've free()'d rent. That's no good. So
we add a new struct rent_data * variable to temporarily store rent->next
for us, so we can use it later. We'll call this new variable 'rent_next'.
REMOVE_FROM_LIST(rent, GET_RENT(ch), next);
rent_next = rent->next; /* Needed after free(). */
free(rent);
rent = rent_next; /* The value before free()'ing. */
Yet, even still, we have a problem. After we remove rent from the list,
can we rely on rent->next being correct? More importantly, should we?
What we're doing is akin to removing 'A' from the alphabet and then
asking, "What comes after 'A' in the alphabet?" The question doesn't make
sense: there is no 'A' in the alphabet. Instead, we want to know what
rent->next is before we remove it from the list. So we move the rent_next
assignment above REMOVE_FROM_LIST.
After all that, our loop functions, but it's ugly. We now have our loop
initialization, condition, and update scattered across a handful of lines.
This situation arises a lot in programming; so much, in fact, that C has
special looping syntax to nicely cover it:
for (initializer; condition; update)
body;
It is executed in the order:
0. initializer
1. condition -- if false, break out from the loop.
2. body
3. update
4. Goto step 1 (*NOT* step 0).
The advantage of this is that it keeps the loop's controls all together,
making it really easy to see when and why and how the loop's body is being
run. Now, putting everything together, our loop has become:
for (rent = GET_RENT(ch); rent; rent = rent_next) {
rent_next = rent->next;
if (rent->hours < 1) {
REMOVE_FROM_LIST(rent, GET_RENT(ch), next);
free(rent);
} else
rent->hours--;
}
I hope that helps. Have fun.
-dak
--
+---------------------------------------------------------------+
| FAQ: http://qsilver.queensu.ca/~fletchra/Circle/list-faq.html |
| Archives: http://post.queensu.ca/listserv/wwwarch/circle.html |
| Newbie List: http://groups.yahoo.com/group/circle-newbies/ |
+---------------------------------------------------------------+
This archive was generated by hypermail 2b30 : 06/25/03 PDT