iterating gi.invent (github issue #1315)

GitHub issue #1315 points out that it is possible for
a downstream function to change an object's nobj field
to point to a completely different chain.

The cited example by @vultur-cadens was:

     for (obj = gi.invent; obj; obj = obj->nobj)
         if (obj->oclass != COIN_CLASS && !obj->cursed && !rn2(5)) {
             curse(obj);
             ++buc_changed;
         }

    curse() drops the weapon with drop_uswapwep(),
        which calls dropx(),
            which calls dropy(),
                which calls dropz(),
                    which calls place_object().

place_object alters the nobj pointer, to point to the floor chain:
    otmp->nobj = fobj;
    fobj = otmp;

The result was that the next loop iteration was then using floor
objects from the floor chain.

This alters several for-loops to use a more consistent approach,
particularly when the obj is being handed off to a function,
where a downstream function might, or might not, alter the nobj
field.

References:

https://github.com/NetHack/NetHack/issues/1315
https://www.reddit.com/r/nethack/comments/1gkc9ub/even_if_you_drop_an_item_before_drinking_from_the/

src/do_wear.c

@@ -3020,7 +3020,7 @@ menu_remarm(int retry)
 staticfn void
 wornarm_destroyed(struct obj *wornarm)
 {
-    struct obj *invobj;
+    struct obj *invobj, *nextobj;
     unsigned wornoid = wornarm->o_id;
 
     /* cancel_don() resets 'afternmv' when appropriate but doesn't reset
@@ -3049,11 +3049,13 @@ wornarm_destroyed(struct obj *wornarm)
        scan invent instead; if already freed it shouldn't be possible to
        have re-used the stale memory for a new item yet but verify o_id
        just in case */
-    for (invobj = gi.invent; invobj; invobj = invobj->nobj)
+    for (invobj = gi.invent; invobj; invobj = nextobj) {
+        nextobj = invobj->nobj;
         if (invobj == wornarm && invobj->o_id == wornoid) {
             useup(wornarm);
             break;
         }
+    }
 }
 
 /*

src/end.c

@@ -1236,14 +1236,15 @@ really_done(int how)
         display_nhwindow(WIN_MESSAGE, FALSE);
 
     if (how != PANICKED) {
-        struct obj *obj;
+        struct obj *obj, *nextobj;
 
         /*
          * This is needed for both inventory disclosure and dumplog.
          * Both are optional, so do it once here instead of duplicating
          * it in both of those places.
          */
-        for (obj = gi.invent; obj; obj = obj->nobj) {
+        for (obj = gi.invent; obj; obj = nextobj) {
+            nextobj = obj->nobj;
             discover_object(obj->otyp, TRUE, FALSE);
             obj->known = obj->bknown = obj->dknown = obj->rknown = 1;
             set_cknown_lknown(obj); /* set flags when applicable */

src/fountain.c

@@ -315,18 +315,20 @@ drinkfountain(void)
             dowaterdemon();
             break;
         case 24: { /* Maybe curse some items */
-            struct obj *obj;
+            struct obj *obj, *nextobj;
             int buc_changed = 0;
 
             pline("This water's no good!");
             morehungry(rn1(20, 11));
             exercise(A_CON, FALSE);
             /* this is more severe than rndcurse() */
-            for (obj = gi.invent; obj; obj = obj->nobj)
+            for (obj = gi.invent; obj; obj = nextobj) {
+                nextobj = obj->nobj;
                 if (obj->oclass != COIN_CLASS && !obj->cursed && !rn2(5)) {
                     curse(obj);
                     ++buc_changed;
                 }
+            }
             if (buc_changed)
                 update_inventory();
             break;
@@ -498,13 +500,14 @@ dipfountain(struct obj *obj)
         pline("An urge to take a bath overwhelms you.");
         {
             long money = money_cnt(gi.invent);
-            struct obj *otmp;
+            struct obj *otmp, *nextobj;
 
             if (money > 10) {
                 /* Amount to lose.  Might get rounded up as fountains don't
                  * pay change... */
                 money = somegold(money) / 10;
-                for (otmp = gi.invent; otmp && money > 0; otmp = otmp->nobj)
+                for (otmp = gi.invent; otmp && money > 0; otmp = nextobj) {
+                    nextobj = otmp->nobj;
                     if (otmp->oclass == COIN_CLASS) {
                         int denomination = objects[otmp->otyp].oc_cost;
                         long coin_loss =
@@ -515,6 +518,7 @@ dipfountain(struct obj *obj)
                         if (!otmp->quan)
                             delobj(otmp);
                     }
+                }
                 You("lost some of your gold in the fountain!");
                 CLEAR_FOUNTAIN_LOOTED(u.ux, u.uy);
                 exercise(A_WIS, FALSE);

src/invent.c

@@ -2224,15 +2224,17 @@ ggetobj(const char *word, int (*fn)(OBJ_P), int mx,
             return 0;
         if (strchr(buf, 'i')) {
             char ailets[1+26+26+1+5+1]; /* $ + a-z + A-Z + # + slop + \0 */
-            struct obj *otmp;
+            struct obj *otmp, *nextobj;
 
             /* applicable inventory letters; if empty, show entire invent */
             ailets[0] = '\0';
             if (ofilter)
-                for (otmp = gi.invent; otmp; otmp = otmp->nobj)
+                for (otmp = gi.invent; otmp; otmp = nextobj) {
+                    nextobj = otmp->nobj;
                     /* strchr() check: limit overflow items to one '#' */
                     if ((*ofilter)(otmp) && !strchr(ailets, otmp->invlet))
                         (void) strkitten(ailets, otmp->invlet);
+                }
             if (display_inventory(ailets, TRUE) == '\033')
                 return 0;
         } else
@@ -3491,7 +3493,7 @@ dispinv_with_action(
     boolean use_inuse_ordering, /* affects sortloot() and header labels */
     const char *alt_label)      /* alternate value for in-use "Accessories" */
 {
-    struct obj *otmp;
+    struct obj *otmp, *nextobj;
     const char *save_accessories = 0;
     char c, save_sortloot = 0;
     unsigned len = lets ? (unsigned) strlen(lets) : 0U;
@@ -3517,9 +3519,11 @@ dispinv_with_action(
     iflags.force_invmenu = save_force_invmenu;
 
     if (c && c != '\033') {
-        for (otmp = gi.invent; otmp; otmp = otmp->nobj)
+        for (otmp = gi.invent; otmp; otmp = nextobj) {
+            nextobj = otmp->nobj;
             if (otmp->invlet == c)
                 return itemactions(otmp);
+        }
     }
     return ECMD_OK;
 }

src/mhitu.c

@@ -1252,7 +1252,7 @@ gulpmu(struct monst *mtmp, struct attack *mattk)
     struct trap *t = t_at(u.ux, u.uy);
     int tmp = d((int) mattk->damn, (int) mattk->damd);
     int tim_tmp;
-    struct obj *otmp2;
+    struct obj *otmp2, *nextobj;
     int i;
     boolean physical_damage = FALSE;
 
@@ -1357,8 +1357,10 @@ gulpmu(struct monst *mtmp, struct attack *mattk)
         u.uswldtim = (unsigned) ((tim_tmp < 2) ? 2 : tim_tmp);
         swallowed(1); /* update the map display, shows hero swallowed */
         if (!flaming(mtmp->data)) {
-            for (otmp2 = gi.invent; otmp2; otmp2 = otmp2->nobj)
+            for (otmp2 = gi.invent; otmp2; otmp2 = nextobj) {
+                nextobj = otmp2->nobj;
                 (void) snuff_lit(otmp2);
+            }
         }
     }
 

src/pray.c

@@ -1267,14 +1267,15 @@ pleased(aligntyp g_align)
             disp.botl = TRUE;
             break;
         case 4: {
-            struct obj *otmp;
+            struct obj *otmp, *nextobj;
             int any = 0;
 
             if (Blind)
                 You_feel("the power of %s.", u_gname());
             else
                 You("are surrounded by %s aura.", an(hcolor(NH_LIGHT_BLUE)));
-            for (otmp = gi.invent; otmp; otmp = otmp->nobj) {
+            for (otmp = gi.invent; otmp; otmp = nextobj) {
+                nextobj = otmp->nobj;
                 if (otmp->cursed
                     && (otmp != uarmh /* [see worst_cursed_item()] */
                         || uarmh->otyp != HELM_OF_OPPOSITE_ALIGNMENT)) {

src/read.c

@@ -2382,7 +2382,7 @@ litroom(
     boolean on,      /* True: make nearby area lit; False: cursed scroll */
     struct obj *obj) /* scroll, spellbook (for spell), or wand of light */
 {
-    struct obj *otmp;
+    struct obj *otmp, *nextobj;
     boolean blessed_effect = (obj && obj->oclass == SCROLL_CLASS
                               && obj->blessed);
     boolean no_op = (u.uswallow || Underwater || Is_waterlevel(&u.uz));
@@ -2400,7 +2400,8 @@ litroom(
          *  Shouldn't this affect all lit objects in the area of effect
          *  rather than just those carried by the hero?
          */
-        for (otmp = gi.invent; otmp; otmp = otmp->nobj) {
+        for (otmp = gi.invent; otmp; otmp = nextobj) {
+            nextobj = otmp->nobj;
             if (otmp->lamplit) {
                 if (!artifact_light(otmp))
                     (void) snuff_lit(otmp);
@@ -2431,7 +2432,8 @@ litroom(
     } else { /* on */
         if (blessed_effect) {
             /* might bless artifact lights; no effect on ordinary lights */
-            for (otmp = gi.invent; otmp; otmp = otmp->nobj) {
+            for (otmp = gi.invent; otmp; otmp = nextobj) {
+                nextobj = otmp->nobj;
                 if (otmp->lamplit && artifact_light(otmp))
                     /* wielded Sunsword or worn gold dragon scales/mail;
                        maybe raise its BUC state if not already blessed */

src/steal.c

@@ -165,12 +165,13 @@ staticfn int
 stealarm(void)
 {
     struct monst *mtmp;
-    struct obj *otmp;
+    struct obj *otmp, *nextobj;
 
     if (!gs.stealoid || !gs.stealmid)
         goto botm;
 
-    for (otmp = gi.invent; otmp; otmp = otmp->nobj) {
+    for (otmp = gi.invent; otmp; otmp = nextobj) {
+        nextobj = otmp->nobj;
         if (otmp->o_id == gs.stealoid) {
             for (mtmp = fmon; mtmp; mtmp = mtmp->nmon) {
                 if (mtmp->m_id == gs.stealmid) {

src/trap.c

@@ -1551,7 +1551,7 @@ trapeffect_rust_trap(
     struct trap *trap,
     unsigned int trflags UNUSED)
 {
-    struct obj *otmp;
+    struct obj *otmp, *nextobj;
 
     if (mtmp == &gy.youmonst) {
         seetrap(trap);
@@ -1583,10 +1583,12 @@ trapeffect_rust_trap(
             pline("%s you!", A_gush_of_water_hits);
             /* note: exclude primary and secondary weapons from splashing
                because cases 1 and 2 target them [via water_damage()] */
-            for (otmp = gi.invent; otmp; otmp = otmp->nobj)
+            for (otmp = gi.invent; otmp; otmp = nextobj) {
+                nextobj = otmp->nobj;
                 if (otmp->lamplit && otmp != uwep
                     && (otmp != uswapwep || !u.twoweap))
                     (void) splash_lit(otmp);
+            }
             if (uarmc)
                 (void) water_damage(uarmc, cloak_simple_name(uarmc), TRUE);
             else if (uarm)
@@ -4764,13 +4766,14 @@ emergency_disrobe(boolean *lostsome)
     int invc = inv_cnt(TRUE);
 
     while (near_capacity() > (Punished ? UNENCUMBERED : SLT_ENCUMBER)) {
-        struct obj *obj, *otmp = (struct obj *) 0;
+        struct obj *obj, *nextobj, *otmp = (struct obj *) 0;
         int i;
 
         /* Pick a random object */
         if (invc > 0) {
             i = rn2(invc);
-            for (obj = gi.invent; obj; obj = obj->nobj) {
+            for (obj = gi.invent; obj; obj = nextobj) {
+                nextobj = obj->nobj;
                 /*
                  * Undroppables are: body armor, boots, gloves,
                  * amulets, and rings because of the time and effort
@@ -6589,7 +6592,7 @@ static const char lava_killer[] = "molten lava";
 boolean
 lava_effects(void)
 {
-    struct obj *obj, *obj2;
+    struct obj *obj, *obj2, *nextobj;
     boolean usurvive, boil_away;
     unsigned protect_oid = 0;
     int burncount = 0, burnmesgcount = 0;
@@ -6616,7 +6619,8 @@ lava_effects(void)
      * emergency save file created before item destruction.
      */
     if (!usurvive) {
-        for (obj = gi.invent; obj; obj = obj->nobj) {
+        for (obj = gi.invent; obj; obj = nextobj) {
+            nextobj = obj->nobj;
             if (obj->in_use) { /* remove_worn_item() sets in_use */
                 /* one item can be protected from burning up [accommodates
                    steal(AMULET_OF_FLYING) -> remove_worn_item() -> fall
@@ -6952,10 +6956,11 @@ trapname(
 void
 ignite_items(struct obj *objchn)
 {
-    struct obj *obj;
+    struct obj *obj, *nextobj;
     boolean bynexthere = (objchn && objchn->where == OBJ_FLOOR);
 
-    for (obj = objchn; obj; obj = bynexthere ? obj->nexthere : obj->nobj) {
+    for (obj = objchn; obj; obj = bynexthere ? obj->nexthere : nextobj) {
+        nextobj = obj->nobj;
         /* ignitable items like lamps and candles will catch fire */
         if (!obj->lamplit && !obj->in_use)
             catch_lit(obj);

src/zap.c

@@ -2626,15 +2626,17 @@ dozap(void)
 staticfn void
 boxlock_invent(struct obj *obj)
 {
-    struct obj *otmp;
+    struct obj *otmp, *nextobj;
     boolean boxing = FALSE;
 
     /* (un)lock carried boxes */
-    for (otmp = gi.invent; otmp; otmp = otmp->nobj)
+    for (otmp = gi.invent; otmp; otmp = nextobj) {
+        nextobj = otmp->nobj;
         if (Is_box(otmp)) {
             (void) boxlock(otmp, obj);
             boxing = TRUE;
         }
+    }
     if (boxing)
         update_inventory(); /* in case any box->lknown has changed */
 }