diff --git a/doc/fixes37.0 b/doc/fixes37.0 index c884f2cd5..b2d17af47 100644 --- a/doc/fixes37.0 +++ b/doc/fixes37.0 @@ -624,6 +624,12 @@ data tracking for #overview was mis-using u.urooms[] and after being in a levels might flag unvisisted rooms as having been visited special damage attacks by the Riders and by fatal-illness inflictors such as Demogorgon did no damage against other monsters, only against the hero +using obj->o_id to control 'random' behavior of a helm of opposite alignment + could potentially be controlled by player when wishing for such +obj->o_id might be set to invalid value 0 when a partly used up stack had a + dummy copy added to a shop's bill or when a bones file was loaded + (in theory that could happen on any system but in practice it could + only happen on a configuration that uses 16-bit ints) Fixes to 3.7.0-x Problems that Were Exposed Via git Repository diff --git a/include/extern.h b/include/extern.h index 98b08e1d9..42ab9dacf 100644 --- a/include/extern.h +++ b/include/extern.h @@ -1329,6 +1329,7 @@ extern struct obj *mkobj(int, boolean); extern int rndmonnum(void); extern boolean bogon_is_pname(char); extern struct obj *splitobj(struct obj *, long); +extern unsigned next_ident(void); extern struct obj *unsplitobj(struct obj *); extern void clear_splitobjs(void); extern void replace_object(struct obj *, struct obj *); diff --git a/src/makemon.c b/src/makemon.c index 067ef4381..28576d41e 100644 --- a/src/makemon.c +++ b/src/makemon.c @@ -837,9 +837,7 @@ clone_mon(struct monst *mon, m2->mextra = (struct mextra *) 0; m2->nmon = fmon; fmon = m2; - m2->m_id = g.context.ident++; - if (!m2->m_id) - m2->m_id = g.context.ident++; /* ident overflowed */ + m2->m_id = next_ident(); m2->mx = mm.x; m2->my = mm.y; @@ -1205,9 +1203,7 @@ makemon(register struct permonst *ptr, mtmp->msleeping = 1; mtmp->nmon = fmon; fmon = mtmp; - mtmp->m_id = g.context.ident++; - if (!mtmp->m_id) - mtmp->m_id = g.context.ident++; /* ident overflowed */ + mtmp->m_id = next_ident(); set_mon_data(mtmp, ptr); /* mtmp->data = ptr; */ if (ptr->msound == MS_LEADER && quest_info(MS_LEADER) == mndx) g.quest_status.leader_m_id = mtmp->m_id; diff --git a/src/mkobj.c b/src/mkobj.c index 1ee4d00e9..490fa37e3 100644 --- a/src/mkobj.c +++ b/src/mkobj.c @@ -358,9 +358,7 @@ copy_oextra(struct obj *obj2, struct obj *obj1) OMONST(obj2)->mextra = (struct mextra *) 0; OMONST(obj2)->nmon = (struct monst *) 0; #if 0 - OMONST(obj2)->m_id = g.context.ident++; - if (OMONST(obj2)->m_id) /* ident overflowed */ - OMONST(obj2)->m_id = g.context.ident++; + OMONST(obj2)->m_id = next_ident(); #endif if (OMONST(obj1)->mextra) copy_mextra(OMONST(obj2), OMONST(obj1)); @@ -421,6 +419,34 @@ splitobj(struct obj *obj, long num) return otmp; } +/* return the value of context.ident and then increment it to be ready for + its next use; used to be simple += 1 so that every value from 1 to N got + used but now has a random increase that skips half of potential values */ +unsigned +next_ident(void) +{ + unsigned res = g.context.ident; + + /* +rnd(2): originally just +1; changed to rnd() to avoid potential + exploit of player using #adjust to split an object stack in a manner + that makes most recent ident%2 known; since #adjust takes no time, + no intervening activity like random creation of a new monster will + take place before next user command; with former +1, o_id%2 of the + next object to be created was knowable and player could make a wish + under controlled circumstances for an item that is affected by the + low bits of its obj->o_id [particularly helm of opposite alignment] */ + g.context.ident += rnd(2); /* ready for next new object or monster */ + + /* if ident has wrapped to 0, force it to be non-zero; if/when it + ever wraps past 0 (unlikely, but possible on a configuration which + uses 16-bit 'int'), just live with that and hope no o_id conflicts + between objects or m_id conflicts between monsters arise */ + if (!g.context.ident) + g.context.ident = rnd(2); + + return res; +} + /* when splitting a stack that has o_id-based shop prices, pick an o_id value for the new stack that will maintain the same price */ static unsigned @@ -436,8 +462,9 @@ nextoid(struct obj *oldobj, struct obj *newobj) ++oid; newdif = oid_price_adjustment(newobj, oid); } while (newdif != olddif && --trylimit >= 0); - g.context.ident = oid + 1; /* ready for next new object */ - return oid; + g.context.ident = oid; /* update 'last ident used' */ + (void) next_ident(); /* increment context.ident for next use */ + return oid; /* caller will use this ident */ } /* try to find the stack obj was split from, then merge them back together; @@ -756,9 +783,7 @@ mksobj(int otyp, boolean init, boolean artif) otmp = newobj(); *otmp = cg.zeroobj; otmp->age = g.moves; - otmp->o_id = g.context.ident++; - if (!otmp->o_id) - otmp->o_id = g.context.ident++; /* ident overflowed */ + otmp->o_id = next_ident(); otmp->quan = 1L; otmp->oclass = let; otmp->otyp = otyp; diff --git a/src/restore.c b/src/restore.c index f82abf688..18bab2d08 100644 --- a/src/restore.c +++ b/src/restore.c @@ -263,7 +263,8 @@ restobjchn(NHFILE* nhfp, boolean frozen) otmp2->nobj = otmp; if (ghostly) { - unsigned nid = g.context.ident++; + unsigned nid = next_ident(); + add_id_mapping(otmp->o_id, nid); otmp->o_id = nid; } @@ -404,7 +405,8 @@ restmonchn(NHFILE* nhfp) mtmp2->nmon = mtmp; if (ghostly) { - unsigned nid = g.context.ident++; + unsigned nid = next_ident(); + add_id_mapping(mtmp->m_id, nid); mtmp->m_id = nid; } diff --git a/src/shk.c b/src/shk.c index f10378130..d54b9b7a2 100644 --- a/src/shk.c +++ b/src/shk.c @@ -2786,7 +2786,7 @@ sub_one_frombill(register struct obj* obj, register struct monst* shkp) otmp = newobj(); *otmp = *obj; otmp->oextra = (struct oextra *) 0; - bp->bo_id = otmp->o_id = g.context.ident++; + bp->bo_id = otmp->o_id = next_ident(); /* g.context.ident++ */ otmp->where = OBJ_FREE; otmp->quan = (bp->bquan -= obj->quan); otmp->owt = 0; /* superfluous */