nethack-gwaa-kiwi/nethack
1
0
Fork 0
Code Pull Requests Activity

fix odd messages caused by buffer re-use

Browse Source 
Reported and diagnosed by entrez:
"The <mon> yanks <two-handed weapon> 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).
This commit is contained in:
PatR
2021-09-15 20:56:06 -07:00
parent fde51720f4
commit e43ec0cef1
4 changed files with 103 additions and 24 deletions
Download Patch File Download Diff File Expand all files Collapse all files

9
src/invent.c
View File

@@ -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;
}
}

107
src/objnam.c
View File

@@ -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 <foo> 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 */