[crossfire] Code cleanlieness and server stability

[Mark Wedel]

Robin Redeker wrote:

> 
> Yes, ok, i've submitted 2 other patches in the last 2 days.
> 
> One question that arose on the last patch:
> 
> We found following code in server/monster.c (around line 1252):
> 
>         else if (item->type == SCROLL && QUERY_FLAG(mon, FLAG_USE_SCROLL)) {
>             if (monster_should_cast_spell(mon, item->inv))
> 
> And in apply.c at void apply_scroll (object *op, object *tmp, int dir):
> 
>         if (!tmp->inv || tmp->inv->type != SPELL) {
>                 new_draw_info (NDI_UNIQUE, 0, op,
>                         "The scroll just doesn't make sense!");
>                 return;
> 
> So we patched it like this:
> 
>         if (item->inv && monster_should_cast_spell(mon, item->inv))

  I think you still need the item->type == SCROLL check in there - otherwise, 
the monster will try to use spellbooks, wands, rods, containers - anything with 
an inventory, as a spell.

  For wands, rods, etc, not an issue, since they do contain spells.  But if the 
monster picks up a chest, item->inv is true, but not a spell object.

> 
> But the question is: How is a scroll with an empty inventory created
> in the first place? We weren't able to find out what behavoir is correct
> and where and how scrolls are created.

  That is one of those mysteries - I've seen it, but can't figure out how it 
happens.

  The problem here is that we know there is an error, but don't know how we got 
into that state.

  One thought I've had, which would use more memory, is to use the glibc 
backtrace() functionality and add code to get_object() that stores away the 
backtrace.  Thus, when we get an object in a funky state, we can hopefully see 
how the object was created (calling structure).  However, this still doesn't 
catch the cases where the object is modified after creation, etc.