From e863583c56f320b121bc11e59e708f55e8976765 Mon Sep 17 00:00:00 2001 From: nhmall Date: Wed, 6 Nov 2024 16:59:51 -0500 Subject: [PATCH] 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 | 6 ++++-- src/end.c | 5 +++-- src/fountain.c | 12 ++++++++---- src/invent.c | 12 ++++++++---- src/mhitu.c | 6 ++++-- src/pray.c | 5 +++-- src/read.c | 8 +++++--- src/steal.c | 5 +++-- src/trap.c | 21 +++++++++++++-------- src/zap.c | 6 ++++-- 10 files changed, 55 insertions(+), 31 deletions(-) diff --git a/src/do_wear.c b/src/do_wear.c index d6c52c4ae..4f20e1b40 100644 --- a/src/do_wear.c +++ b/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; } + } } /* diff --git a/src/end.c b/src/end.c index 1c10e192b..a6e899fa8 100644 --- a/src/end.c +++ b/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 */ diff --git a/src/fountain.c b/src/fountain.c index 848adfbfb..1d4150630 100644 --- a/src/fountain.c +++ b/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); diff --git a/src/invent.c b/src/invent.c index 665e2bd1d..2e696214a 100644 --- a/src/invent.c +++ b/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; } diff --git a/src/mhitu.c b/src/mhitu.c index 8a442faa1..fea0349eb 100644 --- a/src/mhitu.c +++ b/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); + } } } diff --git a/src/pray.c b/src/pray.c index f843ddd9e..82b593d20 100644 --- a/src/pray.c +++ b/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)) { diff --git a/src/read.c b/src/read.c index 5d1a3c92d..1e53e5a2a 100644 --- a/src/read.c +++ b/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 */ diff --git a/src/steal.c b/src/steal.c index 08b4db982..89ac7c841 100644 --- a/src/steal.c +++ b/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) { diff --git a/src/trap.c b/src/trap.c index 9e791e1c5..b75dae7d5 100644 --- a/src/trap.c +++ b/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); diff --git a/src/zap.c b/src/zap.c index fa3795560..37531eeab 100644 --- a/src/zap.c +++ b/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 */ }