Re: GUILD related layout

From: Peter Ajamian (peter@pajamian.dhs.org)
Date: 03/31/01


Web Remedies Network wrote:
>
> db.c:
>
> CASE("Trains") {
>    int numbers[4];
>    sscanf(value, "%d %d %d %d", numbers, numbers+1, numbers+2,
> numbers+3) != 4

Huh?!?  I think you meant to enclose that with an if () statement or
something.  At any rate, you have a comparison that does nothing and
you're missing a ; here.  Also, I always preferred using strtok()/atoi()
to parse input lines.  It allows you to parse them a little at a time so
you don't have to allocate an array like numbers[] to store the whole
thing at once.

>    CREATE(new_train, struct trainer_data, 1);
>    new_train->next = mob_proto[i].mob_specials.trainer;
>    mob_proto[i].mob_specials.trainer->prev = new_train;

I'm willing to bet that this is where your crash is.  What happens if
mob_proto[i].mob_specials.trainer is NULL?  You just tried to dereference
a NULL pointer which is a HUGE no-no.  BTW, why do you want a prev
pointer anyways?

>    new_train->prev = NULL;
>    mob_proto[i].mob_specials.trainer = new_train;
>    if(numbers[0] >= 1 && numbers[0] <= (MAX_SKILLS))
>    new_train->spell_number = numbers[0];
>    else
>    log("Error in processing trainer spell number.");
>    if(numbers[1] >= 1 && numbers[1] <= 100)
>    new_train->percent = numbers[1];
>    else
>    log("Error in processing trainer percent value.");
>    if(numbers[2] >= 0)
>    new_train->cost = numbers[2];
>    else
>    log("Negative value found in cost of trainer spell/skill!");
>    if(numbers[3] >= 0)
>    new_train->qp_cost = numbers[3];
>    else
>    log("Negative value found in quest point cost of trainer skills.");
>   }

Here, you're leaving the new_train as part of the list if the numbers
don't fall in range and you're leaving the values for those numbers
uninitialized.  I would suggest these changes:

Fill in new_train before you add it to the list.  If the any of the
numbers are out of range then log the error, free() new_train, and
return, that wya you don't end up with a messed up structure in your
list.

Again I have to ask why the prev pointer?  Do you really have a need to
walk the list backwards _and_ forwards?  Even if you do want to walk the
list backwards it's useless without a last pointer in mob_specials.  If
you want a fifo there's a better way, like this...

You have a trainer_head and trainer_tail pointer in mob_specials, then to
add a train to the list (note pseudo code, ceartain obvios parts left
out)...

new_train->next = NULL;
if (trainer_tail)
  trainer_tail = trainer_tail->next = new_train;
else
  trainer_head = trainer_tail = new_train;

To pop a train off the list (assumes that there is a train on the list,
you should probably test for this explicitly)...

old_train = trainer_head;
trainer_head = trainer_head->next;
if (!trainer_head)
  trainer_tail = NULL;
free(old_train);

Note that by adding items in this manner you will actually be walking the
list from oldest to newest.  If you want to walk from newest to oldest,
then use the more traditional lifo format (without a tail pointer).  If
you need to walk the list in _both_ directions, then you will need a prev
pointer.

Regards, Peter

--
   +---------------------------------------------------------------+
   | FAQ: http://qsilver.queensu.ca/~fletchra/Circle/list-faq.html |
   | Archives: http://post.queensu.ca/listserv/wwwarch/circle.html |
   +---------------------------------------------------------------+



This archive was generated by hypermail 2b30 : 12/04/01 PST