From: PatR Date: Thu, 16 Sep 2021 03:56:06 +0000 (-0700) Subject: fix odd messages caused by buffer re-use X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=e43ec0cef1cc7c4e639c7de7e1d9ff26c6c8c629;p=nethack fix odd messages caused by buffer re-use Reported and diagnosed by entrez: "The yanks from your corpses!" It became unwielded and that triggered a perm_invent update and such updates reformat entire inventory, so if that contains a dozen or more items it will use all the obuf[] static buffers as least once. In this case, the bullwhip code had plural of "hand" in one of those buffers and by the time it delivered the message which used that, the value had been clobbered. As the diagnosis mentioned, it can be tricky to reproduce since either &obuf[0] or &obuf[PREFIX] might be used and if the value being clobbered didn't overlap, the effect wasn't noticeable. Instead of fixing the bullwhip message, this changes inventory display so that it should no longer churn through all the buffers. It also adds a fixes entry for #K3401, which was already fixed for 3.7 but I hadn't been able to reproduce it for 3.6.x (which I now blame on the PREFIX trickiness). --- diff --git a/doc/fixes37.0 b/doc/fixes37.0 index 78518f303..6c84ddbfc 100644 --- a/doc/fixes37.0 +++ b/doc/fixes37.0 @@ -607,6 +607,16 @@ yet another fix for display problems during restore: if game is saved while instead it would access steed pointer before that has been set up resistances gained from worn or wielded items also protect hero's inventory non-metallic gloves protect worn rings from shock +message "Oops! food rations out of your grasp!" occurred due to perm_invent + in mid-operation overwriting all of xname's/doname's obufs; fixed by + having hold_another_object() defer perm_invent update til done with + all its args (so fixed as a side-effect of "spurious add and remove + perm_invent updates" above, prior to being reported as #K3401) +similar "The ogre lord yanks Cleaver from your corpses!" due to caching the + result from makeplural(body_part(HAND)) then having a perm_invent + update clobber that; fixed by having inventory display release the + obuf used for each item so that the same one will be reused for the + next item, to avoid churning through the whole pool of obufs Fixes to 3.7.0-x Problems that Were Exposed Via git Repository diff --git a/include/extern.h b/include/extern.h index 27ce86eaf..5ea0b0eb6 100644 --- a/include/extern.h +++ b/include/extern.h @@ -1762,6 +1762,7 @@ extern void objects_globals_init(void); /* ### objnam.c ### */ +extern void maybereleaseobuf(char *); extern char *obj_typename(int); extern char *simple_typename(int); extern char *safe_typename(int); diff --git a/src/invent.c b/src/invent.c index 14b222fe9..afd5efe09 100644 --- a/src/invent.c +++ b/src/invent.c @@ -2535,7 +2535,7 @@ display_pickinv( { static const char not_carrying_anything[] = "Not carrying anything"; struct obj *otmp, wizid_fakeobj; - char ilet, ret; + char ilet, ret, *formattedobj; const char *invlet = flags.inv_order; int n, classcount; winid win; /* windows being used */ @@ -2697,9 +2697,14 @@ display_pickinv( any.a_char = ilet; tmpglyph = obj_to_glyph(otmp, rn2_on_display_rng); map_glyphinfo(0, 0, tmpglyph, 0U, &tmpglyphinfo); + formattedobj = doname(otmp); add_menu(win, &tmpglyphinfo, &any, ilet, wizid ? def_oc_syms[(int) otmp->oclass].sym : 0, - ATR_NONE, doname(otmp), MENU_ITEMFLAGS_NONE); + ATR_NONE, formattedobj, MENU_ITEMFLAGS_NONE); + /* doname() uses a static pool of obuf[] output buffers and + we don't want inventory display to overwrite all of them, + so when we've used one we release it for re-use */ + maybereleaseobuf(formattedobj); gotsomething = TRUE; } } diff --git a/src/objnam.c b/src/objnam.c index d8ea61c2b..cd9274c8b 100644 --- a/src/objnam.c +++ b/src/objnam.c @@ -128,6 +128,44 @@ releaseobuf(char *bufp) obufidx = (obufidx - 1 + NUMOBUF) % NUMOBUF; } +/* used by display_pickinv (invent.c, main whole-inventory routine) to + release each successive doname() result in order to try to avoid + clobbering all the obufs when 'perm_invent' is enabled and updated + while one or more obufs have been allocated but not released yet */ +void +maybereleaseobuf(char *obuffer) +{ + releaseobuf(obuffer); + + /* + * An example from 3.6.x where all obufs got clobbered was when a + * monster used a bullwhip to disarm the hero of a two-handed weapon: + * "The ogre lord yanks Cleaver from your corpses!" + | + | hand = body_part(HAND); + | if (use_plural) // switches 'hand' from static buffer to an obuf + | hand = makeplural(hand); + ... + | release_worn_item(); // triggers full inventory update for perm_invent + ... + | pline(..., hand); // the obuf[] for "hands" was clobbered with the + | //+ partial formatting of an item from invent + * + * Another example was from writing a scroll without room in invent to + * hold it after being split from a stack of blank scrolls: + * "Oops! food rations out of your grasp!" + * hold_another_object() was passed 'the(aobjnam(newscroll, "slip"))' + * as an argument and that should have yielded + * "Oops! The scroll of slips out of your grasp!" + * but attempting to add the item to inventory triggered update for + * perm_invent and the result from 'the(...)' was clobbered by partial + * formatting of some inventory item. [It happened in a shop and the + * shk claimed ownership of the new scroll, but that wasn't relevant.] + * That got fixed earlier, by delaying update_inventory() during + * hold_another_object() rather than by avoiding using all the obufs. + */ +} + char * obj_typename(int otyp) { @@ -434,6 +472,7 @@ xname_flags( unsigned cxn_flags) /* bitmask of CXN_xxx values */ { register char *buf; + char *obufp; register int typ = obj->otyp; register struct objclass *ocl = &objects[typ]; int nn = ocl->oc_name_known, omndx = obj->corpsenm; @@ -574,10 +613,18 @@ xname_flags( } else { Strcpy(buf, f->fname); if (pluralize) { - /* ick; already pluralized fruit names - are allowed--we want to try to avoid - adding a redundant plural suffix */ - Strcpy(buf, makeplural(makesingular(buf))); + /* ick: already pluralized fruit names are allowed--we + want to try to avoid adding a redundant plural suffix; + double ick: makesingular() and makeplural() each use + and return an obuf but we don't want any particular + xname() call to consume more than one of those + [note: makeXXX() will be fully evaluated and done with + 'buf' before strcpy() touches its output buffer] */ + Strcpy(buf, obufp = makesingular(buf)); + releaseobuf(obufp); + Strcpy(buf, obufp = makeplural(buf)); + releaseobuf(obufp); + pluralize = FALSE; } } @@ -729,8 +776,11 @@ xname_flags( Sprintf(buf, "glorkum %d %d %d", obj->oclass, typ, obj->spe); break; } - if (pluralize) - Strcpy(buf, makeplural(buf)); + if (pluralize) { + /* (see fruit name handling in case FOOD_CLASS above) */ + Strcpy(buf, obufp = makeplural(buf)); + releaseobuf(obufp); + } /* maybe give some extra information which isn't shown during play */ if (g.program_state.gameover) { @@ -1486,11 +1536,15 @@ corpse_xname( } } - /* it's safe to overwrite our nambuf after an() has copied - its old value into another buffer */ - if (any_prefix) - Strcpy(nambuf, an(nambuf)); + /* it's safe to overwrite our nambuf[] after an() has copied its + old value into another buffer; and once _that_ has been copied, + the obuf[] returned by an() can be made available for re-use */ + if (any_prefix) { + char *obufp; + Strcpy(nambuf, obufp = an(nambuf)); + releaseobuf(obufp); + } return nambuf; } @@ -1949,10 +2003,12 @@ Ysimple_name2(struct obj* obj) char * simpleonames(struct obj* obj) { - char *simpleoname = minimal_xname(obj); + char *obufp, *simpleoname = minimal_xname(obj); - if (obj->quan != 1L) - simpleoname = makeplural(simpleoname); + if (obj->quan != 1L) { + simpleoname = makeplural(obufp = simpleoname); + releaseobuf(obufp); + } return simpleoname; } @@ -1960,7 +2016,7 @@ simpleonames(struct obj* obj) char * ansimpleoname(struct obj* obj) { - char *simpleoname = simpleonames(obj); + char *obufp, *simpleoname = simpleonames(obj); int otyp = obj->otyp; /* prefix with "the" if a unique item, or a fake one imitating same, @@ -1969,12 +2025,16 @@ ansimpleoname(struct obj* obj) if (otyp == FAKE_AMULET_OF_YENDOR) otyp = AMULET_OF_YENDOR; if (objects[otyp].oc_unique - && !strcmp(simpleoname, OBJ_NAME(objects[otyp]))) - return the(simpleoname); - - /* simpleoname is singular if quan==1, plural otherwise */ - if (obj->quan == 1L) - simpleoname = an(simpleoname); + && !strcmp(simpleoname, OBJ_NAME(objects[otyp]))) { + /* the() will allocate another obuf[]; we want to avoid using two */ + Strcpy(simpleoname, obufp = the(simpleoname)); + releaseobuf(obufp); + } else if (obj->quan == 1L) { + /* simpleoname[] is singular if quan==1, plural otherwise; + an() will allocate another obuf[]; we want to avoid using two */ + Strcpy(simpleoname, obufp = an(simpleoname)); + releaseobuf(obufp); + } return simpleoname; } @@ -1982,9 +2042,12 @@ ansimpleoname(struct obj* obj) char * thesimpleoname(struct obj* obj) { - char *simpleoname = simpleonames(obj); + char *obufp, *simpleoname = simpleonames(obj); - return the(simpleoname); + /* the() will allocate another obuf[]; we want to avoid using two */ + Strcpy(simpleoname, obufp = the(simpleoname)); + releaseobuf(obufp); + return simpleoname; } /* artifact's name without any object type or known/dknown/&c feedback */