From 83e35a73bd818032bd606966cf0fdbd9d9463b51 Mon Sep 17 00:00:00 2001 From: PatR Date: Tue, 18 Dec 2018 02:24:19 -0800 Subject: [PATCH 1/5] remove dead code from parse() From Jessie's old static analysis report. 'prezero' was used in 3.4.3 when processing a count for 'multi', but count handling is now done in a separate routine and 'prezero' in parse() never changes value, so get rid of it. --- src/cmd.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/cmd.c b/src/cmd.c index 0cbcaba66..f05787fe3 100644 --- a/src/cmd.c +++ b/src/cmd.c @@ -1,4 +1,4 @@ -/* NetHack 3.6 cmd.c $NHDT-Date: 1544920233 2018/12/16 00:30:33 $ $NHDT-Branch: win-minor $:$NHDT-Revision: 1.321 $ */ +/* NetHack 3.6 cmd.c $NHDT-Date: 1545128652 2018/12/18 10:24:12 $ $NHDT-Branch: NetHack-3.6.2-beta01 $:$NHDT-Revision: 1.322 $ */ /* Copyright (c) Stichting Mathematisch Centrum, Amsterdam, 1985. */ /*-Copyright (c) Robert Patrick Rankin, 2013. */ /* NetHack may be freely redistributed. See license for details. */ @@ -5544,7 +5544,6 @@ parse() static char in_line[COLNO]; #endif register int foo; - boolean prezero = FALSE; iflags.in_parse = TRUE; multi = 0; @@ -5617,8 +5616,6 @@ parse() in_line[2] = 0; } clear_nhwindow(WIN_MESSAGE); - if (prezero) - in_line[0] = Cmd.spkeys[NHKF_ESC]; iflags.in_parse = FALSE; return in_line; From 16e78e5b608077cdaef38f5acff546307da911f6 Mon Sep 17 00:00:00 2001 From: PatR Date: Tue, 18 Dec 2018 02:44:21 -0800 Subject: [PATCH 2/5] plug potential open file leak in checkfile() Another item from static analysis. If an internal error ever caused the "bad do_look buffer" warning from checkfile(), open file 'data' would not be closed. (The bug in checkfile()'s caller which prompted that check was fixed long go.) An alternate fix would be to move the input buffer check to before the file is opened, but verifying the file first seems worthwhile. --- src/pager.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/pager.c b/src/pager.c index 22df977e4..6982860c9 100644 --- a/src/pager.c +++ b/src/pager.c @@ -1,4 +1,4 @@ -/* NetHack 3.6 pager.c $NHDT-Date: 1543185072 2018/11/25 22:31:12 $ $NHDT-Branch: NetHack-3.6.2-beta01 $:$NHDT-Revision: 1.139 $ */ +/* NetHack 3.6 pager.c $NHDT-Date: 1545129848 2018/12/18 10:44:08 $ $NHDT-Branch: NetHack-3.6.2-beta01 $:$NHDT-Revision: 1.142 $ */ /* Copyright (c) Stichting Mathematisch Centrum, Amsterdam, 1985. */ /*-Copyright (c) Robert Patrick Rankin, 2018. */ /* NetHack may be freely redistributed. See license for details. */ @@ -527,14 +527,14 @@ char *supplemental_name; fp = dlb_fopen(DATAFILE, "r"); if (!fp) { - pline("Cannot open data file!"); + pline("Cannot open 'data' file!"); return; } /* If someone passed us garbage, prevent fault. */ if (!inp || strlen(inp) > (BUFSZ - 1)) { impossible("bad do_look buffer passed (%s)!", !inp ? "null" : "too long"); - return; + goto checkfile_done; } /* To prevent the need for entries in data.base like *ngel to account From 39d85a5ce7f26670fdf91db1b595e69a3f5d7ca1 Mon Sep 17 00:00:00 2001 From: PatR Date: Tue, 18 Dec 2018 03:01:50 -0800 Subject: [PATCH 3/5] life support for comatose code in explmu() Static analysis notices that if (physical_damage) tmp = Maybe_Half_Phys(tmp); will never pass the test because all code paths leading to it set 'physical_damage' to False. Instead of getting rid of it, add a fake case that leaves that True. --- src/mhitu.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/mhitu.c b/src/mhitu.c index 85293d57b..67966db5f 100644 --- a/src/mhitu.c +++ b/src/mhitu.c @@ -1,4 +1,4 @@ -/* NetHack 3.6 mhitu.c $NHDT-Date: 1540767817 2018/10/28 23:03:37 $ $NHDT-Branch: NetHack-3.6.2-beta01 $:$NHDT-Revision: 1.159 $ */ +/* NetHack 3.6 mhitu.c $NHDT-Date: 1545130893 2018/12/18 11:01:33 $ $NHDT-Branch: NetHack-3.6.2-beta01 $:$NHDT-Revision: 1.160 $ */ /* Copyright (c) Stichting Mathematisch Centrum, Amsterdam, 1985. */ /*-Copyright (c) Robert Patrick Rankin, 2012. */ /* NetHack may be freely redistributed. See license for details. */ @@ -1989,12 +1989,12 @@ boolean ufound; if (mtmp->mcan) return 0; - if (!ufound) + if (!ufound) { pline("%s explodes at a spot in %s!", canseemon(mtmp) ? Monnam(mtmp) : "It", levl[mtmp->mux][mtmp->muy].typ == WATER ? "empty water" : "thin air"); - else { + } else { int tmp = d((int) mattk->damn, (int) mattk->damd); boolean not_affected = defends((int) mattk->adtyp, uwep); @@ -2012,6 +2012,13 @@ boolean ufound; case AD_ELEC: physical_damage = FALSE; not_affected |= Shock_resistance; + goto common; + case AD_PHYS: + /* there aren't any exploding creatures with AT_EXPL attack + for AD_PHYS damage but there might be someday; without this, + static analysis complains that 'physical_damage' is always + False when tested below; it's right, but having that in + place means one less thing to update if AD_PHYS gets added */ common: if (!not_affected) { From f2fe193b03763241dbbb0a154e79c90f734966a4 Mon Sep 17 00:00:00 2001 From: PatR Date: Tue, 18 Dec 2018 03:13:41 -0800 Subject: [PATCH 4/5] suppress dead code in move_special() This dead code was already dead in 3.4.3; I didn't look any further back. --- src/priest.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/priest.c b/src/priest.c index 4a3dd185d..d97886c73 100644 --- a/src/priest.c +++ b/src/priest.c @@ -1,4 +1,4 @@ -/* NetHack 3.6 priest.c $NHDT-Date: 1501725407 2017/08/03 01:56:47 $ $NHDT-Branch: NetHack-3.6.0 $:$NHDT-Revision: 1.44 $ */ +/* NetHack 3.6 priest.c $NHDT-Date: 1545131519 2018/12/18 11:11:59 $ $NHDT-Branch: NetHack-3.6.2-beta01 $:$NHDT-Revision: 1.45 $ */ /* Copyright (c) Izchak Miller, Steve Linhart, 1989. */ /* NetHack may be freely redistributed. See license for details. */ @@ -53,7 +53,9 @@ register xchar omx, omy, gx, gy; coord poss[9]; long info[9]; long allowflags; +#if 0 /* dead code; see below */ struct obj *ib = (struct obj *) 0; +#endif if (omx == gx && omy == gy) return 0; @@ -121,6 +123,7 @@ pick_move: newsym(nix, niy); if (mtmp->isshk && !in_his_shop && inhishop(mtmp)) check_special_room(FALSE); +#if 0 /* dead code; maybe someday someone will track down why... */ if (ib) { if (cansee(mtmp->mx, mtmp->my)) pline("%s picks up %s.", Monnam(mtmp), @@ -128,6 +131,7 @@ pick_move: obj_extract_self(ib); (void) mpickobj(mtmp, ib); } +#endif return 1; } return 0; From b5ce81111c3ebff0f77b964d13e3541165db68f0 Mon Sep 17 00:00:00 2001 From: PatR Date: Tue, 18 Dec 2018 03:24:38 -0800 Subject: [PATCH 5/5] plug open file leaks for rumors and oracles If the rumors file or oralces file got opened successfully but had bad data, it wouldn't be closed. --- src/rumors.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/rumors.c b/src/rumors.c index a0f367952..033a0126c 100644 --- a/src/rumors.c +++ b/src/rumors.c @@ -1,4 +1,4 @@ -/* NetHack 3.6 rumors.c $NHDT-Date: 1542422933 2018/11/17 02:48:53 $ $NHDT-Branch: NetHack-3.6.2-beta01 $:$NHDT-Revision: 1.30 $ */ +/* NetHack 3.6 rumors.c $NHDT-Date: 1545132266 2018/12/18 11:24:26 $ $NHDT-Branch: NetHack-3.6.2-beta01 $:$NHDT-Revision: 1.34 $ */ /* Copyright (c) Stichting Mathematisch Centrum, Amsterdam, 1985. */ /*-Copyright (c) Robert Patrick Rankin, 2012. */ /* NetHack may be freely redistributed. See license for details. */ @@ -183,13 +183,15 @@ boolean exclude_cookie; void rumor_check() { - dlb *rumors; + dlb *rumors = 0; winid tmpwin; char *endp, line[BUFSZ], xbuf[BUFSZ], rumor_buf[BUFSZ]; if (true_rumor_size < 0L) { /* we couldn't open RUMORFILE */ no_rumors: pline("rumors not accessible."); + if (rumors) + (void) dlb_fclose(rumors); return; } @@ -423,11 +425,10 @@ outoracle(special, delphi) boolean special; boolean delphi; { - char line[COLNO]; - char *endp; + winid tmpwin; dlb *oracles; int oracle_idx; - char xbuf[BUFSZ]; + char *endp, line[COLNO], xbuf[BUFSZ]; /* early return if we couldn't open ORACLEFILE on previous attempt, or if all the oracularities are already exhausted */ @@ -437,17 +438,16 @@ boolean delphi; oracles = dlb_fopen(ORACLEFILE, "r"); if (oracles) { - winid tmpwin; if (oracle_flg == 0) { /* if this is the first outoracle() */ init_oracles(oracles); oracle_flg = 1; if (oracle_cnt == 0) - return; + goto close_oracles; } /* oracle_loc[0] is the special oracle; oracle_loc[1..oracle_cnt-1] are normal ones */ if (oracle_cnt <= 1 && !special) - return; /*(shouldn't happen)*/ + goto close_oracles; /*(shouldn't happen)*/ oracle_idx = special ? 0 : rnd((int) oracle_cnt - 1); (void) dlb_fseek(oracles, (long) oracle_loc[oracle_idx], SEEK_SET); if (!special) /* move offset of very last one into this slot */ @@ -470,6 +470,7 @@ boolean delphi; } display_nhwindow(tmpwin, TRUE); destroy_nhwindow(tmpwin); + close_oracles: (void) dlb_fclose(oracles); } else { couldnt_open_file(ORACLEFILE);