From e9624f25837e1d6c4ee6b0e0b873b0b3db14b71b Mon Sep 17 00:00:00 2001 From: PatR Date: Thu, 6 Dec 2018 17:27:36 -0800 Subject: [PATCH] fix #H7686 - destroy_item()'s inventory traversal Inventory traversal can be disrupted when items being traversed are able to change inventory. I've lost track of how many times this sort of thing has been discovered. Report claimed that boiled potion of polymorph caused transformation which resulted in dropped weapon and dropped or destroyed worn armor. That was evidently a guess; potionbreathe() for that potion only abuses constitution. The traceback showed 'you_were()' was involved. Boiled potion of unholy water triggers human-to-beast transformation of hero inflicted with lycanthropy, yielding similar situation. I didn't notice anything unusual when reproducing this but inventory was definitely vulnerable. My 'one line' fixes entries are steadily getting to be more verbose; I may have to go back to 'fix bug'. :-} --- doc/fixes36.2 | 8 ++++++++ src/zap.c | 39 ++++++++++++++++++++++++++++++--------- 2 files changed, 38 insertions(+), 9 deletions(-) diff --git a/doc/fixes36.2 b/doc/fixes36.2 index f87856d44..7f0673f39 100644 --- a/doc/fixes36.2 +++ b/doc/fixes36.2 @@ -243,6 +243,14 @@ level change after being interruped locking or unlocking a container might if a restore attempt failed and a new game was started instead, it would use stale context from old game if restoration got far enough to load that if hero survives turning into slime (life-saving), survive as a green slime +hero hit by something that causes inventory items to be destroyed with loss of + any of those causing other inventory items to be dropped or destroyed, + inventory traversal became unreliable (known sequence: potions hit by + fire then breahing vapor from boiled unholy water triggering were + transformation to beast form; possible sequence: ring of levitation + blasted by lightning and dropping hero onto fire trap); [3.6.1 fixed a + similar problem with more obvious symptom, an "object lost" panic when + the unholy water was wielded; the fix for that wasn't general enough] Fixes to Post-3.6.1 Problems that Were Exposed Via git Repository diff --git a/src/zap.c b/src/zap.c index 5a5a67c34..8f7adf338 100644 --- a/src/zap.c +++ b/src/zap.c @@ -1,4 +1,4 @@ -/* NetHack 3.6 zap.c $NHDT-Date: 1543744276 2018/12/02 09:51:16 $ $NHDT-Branch: NetHack-3.6.2-beta01 $:$NHDT-Revision: 1.299 $ */ +/* NetHack 3.6 zap.c $NHDT-Date: 1544146046 2018/12/07 01:27:26 $ $NHDT-Branch: NetHack-3.6.2-beta01 $:$NHDT-Revision: 1.300 $ */ /* Copyright (c) Stichting Mathematisch Centrum, Amsterdam, 1985. */ /*-Copyright (c) Robert Patrick Rankin, 2013. */ /* NetHack may be freely redistributed. See license for details. */ @@ -4719,15 +4719,32 @@ void destroy_item(osym, dmgtyp) register int osym, dmgtyp; { - register struct obj *obj, *obj2; + register struct obj *obj; int dmg, xresist, skip; long i, cnt, quan; int dindx; const char *mult; boolean physical_damage; - for (obj = invent; obj; obj = obj2) { - obj2 = obj->nobj; + /* + * Sometimes destroying an item can change inventory aside from the + * item itself (cited case was a potion of polymorph; when destroyed, + * potion_breathe() caused hero to transform and that resulted in + * destruction of some worn armor). Unlike other uses of the object + * bybass mechanism, destroy_item() can be called multiple times for + * same event. So we have to explicitly clear it before each use and + * hope no other section of code expects it to retain previous value. + * + * FIXME? Destruction of a ring of levitation could drop hero onto + * a fire trap which could destroy other items and we'll get called + * recursively. This should still work, but items beyond the ring + * which survive the fire will be marked as already processed by the + * inner call, so will always survive the remainder of the outer call + * instead of being subjected to original chance of destruction. + */ + bypass_objlist(invent, FALSE); /* clear bypass bit for invent */ + + while ((obj = nxt_unbypassed_obj(invent)) != 0) { physical_damage = FALSE; if (obj->oclass != osym) continue; /* test only objs of type osym */ @@ -4820,6 +4837,7 @@ register int osym, dmgtyp; skip++; break; } + if (!skip) { if (obj->in_use) --quan; /* one will be used up elsewhere */ @@ -4852,9 +4870,9 @@ register int osym, dmgtyp; for (i = 0; i < cnt; i++) useup(obj); if (dmg) { - if (xresist) + if (xresist) { You("aren't hurt!"); - else { + } else { const char *how = destroy_strings[dindx][2]; boolean one = (cnt == 1L); @@ -4877,7 +4895,7 @@ destroy_mitem(mtmp, osym, dmgtyp) struct monst *mtmp; int osym, dmgtyp; { - struct obj *obj, *obj2; + struct obj *obj; int skip, tmp = 0; long i, cnt, quan; int dindx; @@ -4889,8 +4907,11 @@ int osym, dmgtyp; } vis = canseemon(mtmp); - for (obj = mtmp->minvent; obj; obj = obj2) { - obj2 = obj->nobj; + + /* see destroy_item(); object destruction could disrupt inventory list */ + bypass_objlist(mtmp->minvent, FALSE); /* clear bypass bit for invent */ + + while ((obj = nxt_unbypassed_obj(mtmp->minvent)) != 0) { if (obj->oclass != osym) continue; /* test only objs of type osym */ skip = 0;