From 7a533a911c32ad6733e0e197f89694638990ad4b Mon Sep 17 00:00:00 2001 From: PatR Date: Fri, 22 Dec 2023 17:48:51 -0800 Subject: [PATCH] enhance timer sanity checks a bit Four kinds of timers are defined but only two have ever been used. Have sanity checking complain if the other two occur or if 'kind' doesn't match any of the four. Also, replacing a perfectly normal use of isok() with an inline test just to pacify static analysis feels like a slippery slope, so handle that a little differently. I reordered the shrink_glob timer to put all object timers together. Unfortunately that warrants incrementing EDITLEVEL which invalidates existing save files. --- include/patchlevel.h | 4 +-- include/timeout.h | 12 +++++-- src/timeout.c | 79 +++++++++++++++++++++++++++++++++++--------- 3 files changed, 74 insertions(+), 21 deletions(-) diff --git a/include/patchlevel.h b/include/patchlevel.h index 79d5b0fd9..c92092837 100644 --- a/include/patchlevel.h +++ b/include/patchlevel.h @@ -1,4 +1,4 @@ -/* NetHack 3.7 patchlevel.h $NHDT-Date: 1702079342 2023/12/08 23:49:02 $ $NHDT-Branch: NetHack-3.7 $:$NHDT-Revision: 1.247 $ */ +/* NetHack 3.7 patchlevel.h $NHDT-Date: 1703294869 2023/12/23 01:27:49 $ $NHDT-Branch: NetHack-3.7 $:$NHDT-Revision: 1.249 $ */ /* Copyright (c) Stichting Mathematisch Centrum, Amsterdam, 1985. */ /*-Copyright (c) Michael Allison, 2012. */ /* NetHack may be freely redistributed. See license for details. */ @@ -17,7 +17,7 @@ * Incrementing EDITLEVEL can be used to force invalidation of old bones * and save files. */ -#define EDITLEVEL 94 +#define EDITLEVEL 95 /* * Development status possibilities. diff --git a/include/timeout.h b/include/timeout.h index f01139936..a65f2bf51 100644 --- a/include/timeout.h +++ b/include/timeout.h @@ -1,4 +1,4 @@ -/* NetHack 3.7 timeout.h $NHDT-Date: 1596498564 2020/08/03 23:49:24 $ $NHDT-Branch: NetHack-3.7 $:$NHDT-Revision: 1.13 $ */ +/* NetHack 3.7 timeout.h $NHDT-Date: 1703294874 2023/12/23 01:27:54 $ $NHDT-Branch: NetHack-3.7 $:$NHDT-Revision: 1.22 $ */ /* Copyright 1994, Dean Luick */ /* NetHack may be freely redistributed. See license for details. */ @@ -22,8 +22,14 @@ enum timer_type { #define RANGE_GLOBAL 1 /* save/restore timers following global play */ /* - * Timeout functions. Add a define here, then put it in the table + * Timeout functions. Add an enum here, then put it in the table * in timeout.c. "One more level of indirection will fix everything." + * + * Note: if any are inserted, removed, or reordered then EDITLEVEL + * needs to be incremented because timeout indices get written into save + * and bones files if any timers are present while saving. (Adding new + * ones at the end isn't restricted this way since new indices won't be + * present in old data.) */ enum timeout_types { ROT_ORGANIC = 0, /* for buried organics */ @@ -33,8 +39,8 @@ enum timeout_types { BURN_OBJECT, HATCH_EGG, FIG_TRANSFORM, - MELT_ICE_AWAY, SHRINK_GLOB, + MELT_ICE_AWAY, NUM_TIME_FUNCS }; diff --git a/src/timeout.c b/src/timeout.c index 14a5f8d13..315e82167 100644 --- a/src/timeout.c +++ b/src/timeout.c @@ -1,4 +1,4 @@ -/* NetHack 3.7 timeout.c $NHDT-Date: 1658390077 2022/07/21 07:54:37 $ $NHDT-Branch: NetHack-3.7 $:$NHDT-Revision: 1.142 $ */ +/* NetHack 3.7 timeout.c $NHDT-Date: 1703294874 2023/12/23 01:27:54 $ $NHDT-Branch: NetHack-3.7 $:$NHDT-Revision: 1.167 $ */ /* Copyright (c) Stichting Mathematisch Centrum, Amsterdam, 1985. */ /*-Copyright (c) Robert Patrick Rankin, 2018. */ /* NetHack may be freely redistributed. See license for details. */ @@ -1872,8 +1872,11 @@ typedef struct { #endif } ttable; -/* table of timeout functions */ +/* + * Table of timeout functions, listed in order of enum timeout_types: + */ static const ttable timeout_funcs[NUM_TIME_FUNCS] = { + /* object timers */ TTAB(rot_organic, (timeout_proc) 0, "rot_organic"), TTAB(rot_corpse, (timeout_proc) 0, "rot_corpse"), TTAB(revive_mon, (timeout_proc) 0, "revive_mon"), @@ -1881,8 +1884,10 @@ static const ttable timeout_funcs[NUM_TIME_FUNCS] = { TTAB(burn_object, cleanup_burn, "burn_object"), TTAB(hatch_egg, (timeout_proc) 0, "hatch_egg"), TTAB(fig_transform, (timeout_proc) 0, "fig_transform"), - TTAB(melt_ice_away, (timeout_proc) 0, "melt_ice_away"), TTAB(shrink_glob, (timeout_proc) 0, "shrink_glob"), + /* level timers */ + TTAB(melt_ice_away, (timeout_proc) 0, "melt_ice_away"), + /* currently no monster or global timers */ }; #undef TTAB @@ -2011,31 +2016,73 @@ void timer_sanity_check(void) { timer_element *curr; + unsigned long t_id; + coordxy x, y; - /* this should be much more complete */ for (curr = gt.timer_base; curr; curr = curr->next) { - if (curr->kind == TIMER_OBJECT) { + t_id = curr->tid; + switch (curr->kind) { + case TIMER_OBJECT: { + /* TODO? verify that the timer type is attached to applicable + object (egg for hatch, glob for shrink, and so forth) */ struct obj *obj = curr->arg.a_obj; + char *obj_adr = fmt_ptr((genericptr_t) obj); + int owhere = obj->where; if (obj->timed == 0) { - impossible("timer sanity: untimed obj %s, timer %ld", - fmt_ptr((genericptr_t) obj), curr->tid); + impossible("timer sanity: untimed obj %s, timer %lu", + obj_adr, t_id); } - } else if (curr->kind == TIMER_LEVEL) { - long where = curr->arg.a_long; - coordxy x = (coordxy) ((where >> 16) & 0xFFFF), - y = (coordxy) (where & 0xFFFF); + x = y = 0; + if (owhere == OBJ_MIGRATING + || (owhere == OBJ_MINVENT && !mon_is_local(obj->ocarry))) { + ; /* not able to validate location so skip checks */ + } else if (!get_obj_location(obj, &x, &y, + CONTAINED_TOO | BURIED_TOO)) { + /* free? or on a shop's used-up bill? */ + impossible( + "timer sanity: can't locate obj %s [where=%d], timer %lu", + obj_adr, owhere, t_id); + } else if (!isok(x, y)) { + impossible( + "timer sanity: obj %s [where=%d] located at <%d,%d>, timer %lu", + obj_adr, owhere, x, y, t_id); + } + break; + } + case TIMER_MONSTER: + impossible("timer sanity: unexpected monster timer %lu", t_id); + break; + case TIMER_LEVEL: { + long lwhere = curr->arg.a_long; + + x = (coordxy) ((lwhere >> 16) & 0xFFFF); + y = (coordxy) (lwhere & 0xFFFF); + if (isok(x, y)) { + /* replicate isok() in order to convince static analysis + that the decoding via '& 0xFFFF' hasn't produced a value + too big for levl[][] and that the cast to a narrower type + hasn't intruded on the sign bit to yield a negative value; + the analyzer isn't aware that isok() filters such things */ + assert(x > 0 && x < COLNO && y >= 0 && y < ROWNO); - /* instead of isok(x,y), so static analyzer follows along better */ - if (x > 0 && x < COLNO && y >= 0 && y < ROWNO) { if (curr->func_index == MELT_ICE_AWAY && !is_ice(x, y)) impossible( - "timer sanity: melt timer %lu on non-ice %d <%d,%d>", - curr->tid, levl[x][y].typ, x, y); + "timer sanity: melt timer %lu on non-ice %d <%d,%d>", + t_id, levl[x][y].typ, x, y); } else { impossible("timer sanity: spot timer %lu at <%d,%d>", - curr->tid, x, y); + t_id, x, y); } + break; + } + case TIMER_GLOBAL: + impossible("timer sanity: unexpected global timer %lu", t_id); + break; + default: + impossible("timer sanity: unknown timer %lu, type: %d", + t_id, curr->kind); + break; } } }