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

Qt tombstone bugs

Browse Source 
Infrastructure bits:  Qt tombstone uses a short buffer; make sure that
the plname value fits instead of relying on snprintf() to truncate it.
A warning about gold, if any, was iffy but this should guarantee no
reason for future complaint.  Year was safe but a compiler sensitive
to buffer overflows wouldn't know that.

Actual bugs:  Qt used money in inventory for gold amount on tombstone;
that overlooks gold in containers and will be 0 by tombstone stage if
bones get saved.  Year was recalculated from current date+time instead
of using the value that gets passed in--blindly flagging that variable
as UNUSED was a mistake.
This commit is contained in:
PatR
2020-08-20 16:56:50 -07:00
parent 61b86b8838
commit c062822a7c
6 changed files with 85 additions and 49 deletions
Download Patch File Download Diff File Expand all files Collapse all files
doc/fixes37.0
+7 -1
View File
@@ -1,4 +1,4 @@
NHDT-Branch: NetHack-3.7 $:$NHDT-Revision: 1.288 $ $NHDT-Date: 1597707740 2020/08/17 23:42:20 $ NHDT-Branch: NetHack-3.7 $:$NHDT-Revision: 1.289 $ $NHDT-Date: 1597967807 2020/08/20 23:56:47 $
General Fixes and Modified Features General Fixes and Modified Features
----------------------------------- -----------------------------------
@@ -383,6 +383,12 @@ Qt: "paper doll" subset of persistent inventory has undergone several changes:
first active light source in a previously unused slot on lower right; first active light source in a previously unused slot on lower right;
show first leash-in-use in a previously unused slot on lower left show first leash-in-use in a previously unused slot on lower left
Qt: paper doll inventory view was inconsistently updated during Hallucination Qt: paper doll inventory view was inconsistently updated during Hallucination
Qt: when hero died, gold on tombstone only included gold in inventory, not
any additional gold inside carried containers; also, inventory gold
will be zero if bones get created for all 3.6.x and for 3.4.x+GOLDOBJ
Qt: tombstone showed newly constructed date instead of the value set up at
time of death; it only shows year but that could be wrong if player
stared at or ignored prior --More-- for long enough on 31 December
Qt+QSX: fix control key Qt+QSX: fix control key
Qt+OSX: rename menu entry "nethack->Preferences..." for invoking nethack's Qt+OSX: rename menu entry "nethack->Preferences..." for invoking nethack's
'O' command to "Game->Run-time options" and entry "Game->Qt settings" 'O' command to "Game->Run-time options" and entry "Game->Qt settings"
include/wincurs.h
+1 -1
View File
@@ -105,7 +105,7 @@ extern void curses_number_pad(int state);
extern void curses_delay_output(void); extern void curses_delay_output(void);
extern void curses_start_screen(void); extern void curses_start_screen(void);
extern void curses_end_screen(void); extern void curses_end_screen(void);
extern void curses_outrip(winid wid, int how); extern void curses_outrip(winid wid, int how, time_t when);
extern void genl_outrip(winid tmpwin, int how, time_t when); extern void genl_outrip(winid tmpwin, int how, time_t when);
extern void curses_preference_update(const char *pref); extern void curses_preference_update(const char *pref);
extern void curs_reset_windows(boolean, boolean); extern void curs_reset_windows(boolean, boolean);
src/rip.c
+18 -18
View File
@@ -1,4 +1,4 @@
/* NetHack 3.7 rip.c $NHDT-Date: 1596498204 2020/08/03 23:43:24 $ $NHDT-Branch: NetHack-3.7 $:$NHDT-Revision: 1.32 $ */ /* NetHack 3.7 rip.c $NHDT-Date: 1597967808 2020/08/20 23:56:48 $ $NHDT-Branch: NetHack-3.7 $:$NHDT-Revision: 1.33 $ */
/* Copyright (c) Stichting Mathematisch Centrum, Amsterdam, 1985. */ /* Copyright (c) Stichting Mathematisch Centrum, Amsterdam, 1985. */
/*-Copyright (c) Robert Patrick Rankin, 2017. */ /*-Copyright (c) Robert Patrick Rankin, 2017. */
/* NetHack may be freely redistributed. See license for details. */ /* NetHack may be freely redistributed. See license for details. */
@@ -60,12 +60,10 @@ static const char *rip_txt[] = {
}; };
#define STONE_LINE_CENT 19 /* char[] element of center of stone face */ #define STONE_LINE_CENT 19 /* char[] element of center of stone face */
#endif /* NH320_DEDICATION */ #endif /* NH320_DEDICATION */
#define STONE_LINE_LEN \ #define STONE_LINE_LEN 16 /* # chars that fit on one line
16 /* # chars that fit on one line \ * (note 1 ' ' border) */
* (note 1 ' ' border) \ #define NAME_LINE 6 /* *char[] line # for player name */
*/ #define GOLD_LINE 7 /* *char[] line # for amount of gold */
#define NAME_LINE 6 /* *char[] line # for player name */
#define GOLD_LINE 7 /* *char[] line # for amount of gold */
#define DEATH_LINE 8 /* *char[] line # for death description */ #define DEATH_LINE 8 /* *char[] line # for death description */
#define YEAR_LINE 12 /* *char[] line # for year */ #define YEAR_LINE 12 /* *char[] line # for year */
@@ -90,9 +88,9 @@ time_t when;
register char **dp; register char **dp;
register char *dpx; register char *dpx;
char buf[BUFSZ]; char buf[BUFSZ];
long year;
register int x; register int x;
int line; int line, year;
long cash;
g.rip = dp = (char **) alloc(sizeof(rip_txt)); g.rip = dp = (char **) alloc(sizeof(rip_txt));
for (x = 0; rip_txt[x]; ++x) for (x = 0; rip_txt[x]; ++x)
@@ -100,13 +98,15 @@ time_t when;
dp[x] = (char *) 0; dp[x] = (char *) 0;
/* Put name on stone */ /* Put name on stone */
Sprintf(buf, "%s", g.plname); Sprintf(buf, "%.*s", (int) STONE_LINE_LEN, g.plname);
buf[STONE_LINE_LEN] = 0;
center(NAME_LINE, buf); center(NAME_LINE, buf);
/* Put $ on stone */ /* Put $ on stone */
Sprintf(buf, "%ld Au", g.done_money); cash = max(g.done_money, 0L);
buf[STONE_LINE_LEN] = 0; /* It could be a *lot* of gold :-) */ /* arbitrary upper limit; practical upper limit is quite a bit less */
if (cash > 999999999L)
cash = 999999999L;
Sprintf(buf, "%ld Au", cash);
center(GOLD_LINE, buf); center(GOLD_LINE, buf);
/* Put together death description */ /* Put together death description */
@@ -114,11 +114,11 @@ time_t when;
/* Put death type on stone */ /* Put death type on stone */
for (line = DEATH_LINE, dpx = buf; line < YEAR_LINE; line++) { for (line = DEATH_LINE, dpx = buf; line < YEAR_LINE; line++) {
register int i, i0;
char tmpchar; char tmpchar;
int i, i0 = (int) strlen(dpx);
if ((i0 = strlen(dpx)) > STONE_LINE_LEN) { if (i0 > STONE_LINE_LEN) {
for (i = STONE_LINE_LEN; ((i0 > STONE_LINE_LEN) && i); i--) for (i = STONE_LINE_LEN; (i > 0) && (i0 > STONE_LINE_LEN); --i)
if (dpx[i] == ' ') if (dpx[i] == ' ')
i0 = i; i0 = i;
if (!i) if (!i)
@@ -135,8 +135,8 @@ time_t when;
} }
/* Put year on stone */ /* Put year on stone */
year = yyyymmdd(when) / 10000L; year = (int) ((yyyymmdd(when) / 10000L) % 10000L);
Sprintf(buf, "%4ld", year); Sprintf(buf, "%4d", year);
center(YEAR_LINE, buf); center(YEAR_LINE, buf);
#ifdef DUMPLOG #ifdef DUMPLOG
win/Qt/qt_menu.cpp
+43 -20
View File
@@ -653,7 +653,7 @@ bool NetHackQtTextWindow::Destroy()
return !isVisible(); return !isVisible();
} }
void NetHackQtTextWindow::UseRIP(int how, time_t when UNUSED) void NetHackQtTextWindow::UseRIP(int how, time_t when)
{ {
// Code from X11 windowport // Code from X11 windowport
#define STONE_LINE_LEN 16 /* # chars that fit on one line */ #define STONE_LINE_LEN 16 /* # chars that fit on one line */
@@ -677,41 +677,63 @@ static char** rip_line=0;
int line; int line;
/* Put name on stone */ /* Put name on stone */
snprintf(rip_line[NAME_LINE], STONE_LINE_LEN+1, "%s", g.plname); (void) snprintf(rip_line[NAME_LINE], STONE_LINE_LEN + 1,
"%.*s", STONE_LINE_LEN, g.plname);
/* Put $ on stone */ /* Put $ on stone;
snprintf(rip_line[GOLD_LINE], STONE_LINE_LEN+1, "%ld Au", money_cnt(g.invent)); to keep things safe and relatively simple, impose an arbitrary
upper limit that's the same for 64 bit and 32 bit configurations
(also 16 bit configurations provided they use 32 bit long); the
upper limit for directly carried gold is somewhat less than 300K
due to carrying capacity, but end-of-game handling has already
added in gold from containers, so the amount could be much more
(simplest case: ~300K four times in a blessed bag of holding, so
~1.2M; in addition to the hassle of getting such a thing set up,
it would need many gold-rich bones levels or wizard mode wishing) */
long cash = std::max(g.done_money, 0L);
/* force less that 10 digits to satisfy elaborate format checking;
it's arbitrary but still way, way more than could ever be needed */
if (cash > 999999999L)
cash = 999999999L;
(void) snprintf(rip_line[GOLD_LINE], STONE_LINE_LEN + 1, "%ld Au", cash);
/* Put together death description */ /* Put together death description */
formatkiller(buf, sizeof buf, how, FALSE); formatkiller(buf, sizeof buf, how, FALSE);
//str_copy(buf, killer, SIZE(buf)); //str_copy(buf, killer, SIZE(buf));
/* Put death type on stone */ /* Put death type on stone */
for (line=DEATH_LINE, dpx = buf; line<YEAR_LINE; line++) { for (line = DEATH_LINE, dpx = buf; line < YEAR_LINE; ++line) {
int i,i0;
char tmpchar; char tmpchar;
int i, i0 = (int) strlen(dpx);
if ( (i0=strlen(dpx)) > STONE_LINE_LEN) { if (i0 > STONE_LINE_LEN) {
for(i = STONE_LINE_LEN; for (i = STONE_LINE_LEN; (i > 0) && (i0 > STONE_LINE_LEN); --i)
((i0 > STONE_LINE_LEN) && i); i--) if (dpx[i] == ' ')
if(dpx[i] == ' ') i0 = i; i0 = i;
if(!i) i0 = STONE_LINE_LEN; if (!i)
i0 = STONE_LINE_LEN;
} }
tmpchar = dpx[i0]; tmpchar = dpx[i0];
dpx[i0] = 0; dpx[i0] = 0;
str_copy(rip_line[line], dpx, STONE_LINE_LEN+1); (void) str_copy(rip_line[line], dpx, STONE_LINE_LEN + 1);
if (tmpchar != ' ') { if (tmpchar != ' ') {
dpx[i0] = tmpchar; dpx[i0] = tmpchar;
dpx= &dpx[i0]; dpx= &dpx[i0];
} else dpx= &dpx[i0+1]; } else {
dpx= &dpx[i0 + 1];
}
} }
/* Put year on stone */ /* Put year on stone;
snprintf(rip_line[YEAR_LINE], STONE_LINE_LEN+1, "%4d", getyear()); 64 bit configuration with 64 bit int is capable of overflowing
STONE_LINE_LEN characters; a compiler might warn about that,
so force a value that it can recognize as fitting within buffer's
range ("%4d" imposes a minimum number of digits, not a maximum) */
int year = (int) ((yyyymmdd(when) / 10000L) % 10000L); /* Y10K bug! */
(void) snprintf(rip_line[YEAR_LINE], STONE_LINE_LEN + 1, "%4d", year);
rip.setLines(rip_line,YEAR_LINE+1); rip.setLines(rip_line, YEAR_LINE + 1);
use_rip = true;
use_rip=true;
} }
void NetHackQtTextWindow::Clear() void NetHackQtTextWindow::Clear()
@@ -820,8 +842,9 @@ void NetHackQtMenuOrTextWindow::StartMenu()
if (!actual) actual=new NetHackQtMenuWindow(parent); if (!actual) actual=new NetHackQtMenuWindow(parent);
actual->StartMenu(); actual->StartMenu();
} }
void NetHackQtMenuOrTextWindow::AddMenu(int glyph, const ANY_P* identifier, char ch, char gch, int attr, void NetHackQtMenuOrTextWindow::AddMenu(int glyph, const ANY_P* identifier,
const QString& str, unsigned itemflags) char ch, char gch, int attr,
const QString& str, unsigned itemflags)
{ {
if (!actual) impossible("AddMenu called before we know if Menu or Text"); if (!actual) impossible("AddMenu called before we know if Menu or Text");
actual->AddMenu(glyph,identifier,ch,gch,attr,str,itemflags); actual->AddMenu(glyph,identifier,ch,gch,attr,str,itemflags);
win/X11/wintext.c
+12 -6
View File
@@ -1,4 +1,4 @@
/* NetHack 3.7 wintext.c $NHDT-Date: 1596498376 2020/08/03 23:46:16 $ $NHDT-Branch: NetHack-3.7 $:$NHDT-Revision: 1.21 $ */ /* NetHack 3.7 wintext.c $NHDT-Date: 1597967808 2020/08/20 23:56:48 $ $NHDT-Branch: NetHack-3.7 $:$NHDT-Revision: 1.22 $ */
/* Copyright (c) Dean Luick, 1992 */ /* Copyright (c) Dean Luick, 1992 */
/* NetHack may be freely redistributed. See license for details. */ /* NetHack may be freely redistributed. See license for details. */
@@ -489,14 +489,20 @@ calculate_rip_text(int how, time_t when)
char buf[BUFSZ]; char buf[BUFSZ];
char *dpx; char *dpx;
int line; int line, year;
long year; long cash;
/* Put name on stone */ /* Put name on stone */
Sprintf(rip_line[NAME_LINE], "%s", g.plname); Sprintf(rip_line[NAME_LINE], "%s", g.plname);
/* Put $ on stone */ /* Put $ on stone */
Sprintf(rip_line[GOLD_LINE], "%ld Au", g.done_money); cash = max(g.done_money, 0L);
/* arbitrary upper limit; practical upper limit is quite a bit less */
if (cash > 999999999L)
cash = 999999999L;
Sprintf(buf, "%ld Au", cash);
Sprintf(rip_line[GOLD_LINE], "%ld Au", cash);
/* Put together death description */ /* Put together death description */
formatkiller(buf, sizeof buf, how, FALSE); formatkiller(buf, sizeof buf, how, FALSE);
@@ -523,8 +529,8 @@ calculate_rip_text(int how, time_t when)
} }
/* Put year on stone */ /* Put year on stone */
year = yyyymmdd(when) / 10000L; year = (int) ((yyyymmdd(when) / 10000L) % 10000L);
Sprintf(rip_line[YEAR_LINE], "%4ld", year); Sprintf(rip_line[YEAR_LINE], "%4d", year);
} }
/* /*
win/curses/cursmain.c
+4 -3
View File
@@ -898,12 +898,13 @@ curses_end_screen()
/* /*
outrip(winid, int) outrip(winid, int)
-- The tombstone code. If you want the traditional code use -- The tombstone code. We use genl_outrip() from rip.c
genl_outrip for the value and check the #if in rip.c. instead of rolling our own.
*/ */
void void
curses_outrip(winid wid UNUSED, curses_outrip(winid wid UNUSED,
int how UNUSED) int how UNUSED,
time_t when UNUSED)
{ {
return; return;
} }