From 9ba0cf2ff02a5f15d34b7f0a0e767b09c9d9a26d Mon Sep 17 00:00:00 2001 From: PatR Date: Thu, 21 Mar 2024 15:28:13 -0700 Subject: [PATCH] fix memory leaks for #quit while in tutorial The new change to reset discoveries and monster-stats when exiting the tutorial used dynamic data which wouldn't be freed if player used #quit and declined to resume the regular game. It turns out that such a leak was already present for start-of-game inventory that gets stashed away during the tutorial. In both cases, it could happen at most once per game so wasn't a big deal as far as memory leaks go. --- include/decl.h | 6 +++ include/extern.h | 29 ++++++++++----- src/decl.c | 5 +++ src/nhlua.c | 97 +++++++++++++++++++++++++++++++----------------- src/save.c | 1 + 5 files changed, 94 insertions(+), 44 deletions(-) diff --git a/include/decl.h b/include/decl.h index d13ebe9e8..a35e1445d 100644 --- a/include/decl.h +++ b/include/decl.h @@ -422,6 +422,12 @@ struct instance_globals_g { /* invent.c */ long glyph_reset_timestamp; + /* nhlua.c */ + boolean gmst_stored; + long gmst_moves; + struct obj *gmst_invent; + genericptr_t *gmst_ubak, *gmst_disco, *gmst_mvitals; + /* pline.c */ struct gamelog_line *gamelog; diff --git a/include/extern.h b/include/extern.h index 0e1a922e8..070dc9886 100644 --- a/include/extern.h +++ b/include/extern.h @@ -1966,20 +1966,29 @@ extern char *get_nh_lua_variables(void); extern void save_luadata(NHFILE *) NONNULLARG1; extern void restore_luadata(NHFILE *) NONNULLARG1; extern int nhl_pcall(lua_State *, int, int, const char *) NONNULLARG1; -extern int nhl_pcall_handle(lua_State *, int, int, const char *, NHL_pcall_action) NONNULLARG1; +extern int nhl_pcall_handle(lua_State *, int, int, const char *, + NHL_pcall_action) NONNULLARG1; extern boolean load_lua(const char *, nhl_sandbox_info *) NONNULLARG12; -ATTRNORETURN extern void nhl_error(lua_State *, const char *) NORETURN NONNULLARG12; +ATTRNORETURN extern void nhl_error(lua_State *, const char *) + NORETURN NONNULLARG12; extern void lcheck_param_table(lua_State *) NONNULLARG1; extern schar get_table_mapchr(lua_State *, const char *) NONNULLARG12; -extern schar get_table_mapchr_opt(lua_State *, const char *, schar) NONNULLARG12; +extern schar get_table_mapchr_opt(lua_State *, const char *, schar) + NONNULLARG12; extern short nhl_get_timertype(lua_State *, int) NONNULLARG1; -extern boolean nhl_get_xy_params(lua_State *, lua_Integer *, lua_Integer *) NONNULLARG123; -extern void nhl_add_table_entry_int(lua_State *, const char *, lua_Integer) NONNULLARG12; -extern void nhl_add_table_entry_char(lua_State *, const char *, char) NONNULLARG12; -extern void nhl_add_table_entry_str(lua_State *, const char *, const char *) NONNULLARG123; -extern void nhl_add_table_entry_bool(lua_State *, const char *, boolean) NONNULLARG12; +extern boolean nhl_get_xy_params(lua_State *, lua_Integer *, lua_Integer *) + NONNULLARG123; +extern void nhl_add_table_entry_int(lua_State *, const char *, lua_Integer) + NONNULLARG12; +extern void nhl_add_table_entry_char(lua_State *, const char *, char) + NONNULLARG12; +extern void nhl_add_table_entry_str(lua_State *, const char *, const char *) + NONNULLARG123; +extern void nhl_add_table_entry_bool(lua_State *, const char *, boolean) + NONNULLARG12; extern void nhl_add_table_entry_region(lua_State *, const char *, - coordxy, coordxy, coordxy, coordxy) NONNULLARG12; + coordxy, coordxy, coordxy, coordxy) + NONNULLARG12; extern schar splev_chr2typ(char); extern schar check_mapchr(const char *) NO_NNARGS; extern int get_table_int(lua_State *, const char *) NONNULLARG12; @@ -1996,8 +2005,10 @@ extern int get_table_option(lua_State *, const char *, const char *, /* extern int str_lines_max_width(const char *); */ extern const char *get_lua_version(void); extern void nhl_pushhooked_open_table(lua_State *L) NONNULLARG1; +extern void free_tutorial(void); extern void tutorial(boolean); #endif /* !CROSSCOMPILE || CROSSCOMPILE_TARGET */ + #endif /* MAKEDEFS_C MDLIB_C CPPREGEX_C */ /* ### {cpp,pmatch,posix}regex.c ### */ diff --git a/src/decl.c b/src/decl.c index 7b11846f8..6b3cda881 100644 --- a/src/decl.c +++ b/src/decl.c @@ -404,6 +404,11 @@ static const struct instance_globals_g g_init_g = { { UNDEFINED_VALUES }, /* gems */ /* invent.c */ 0L, /* glyph_reset_timestamp */ + /* nhlua.c */ + FALSE, /* gmst_stored */ + 0L, /* gmst_moves */ + NULL, /* gmst_invent */ + NULL, NULL, NULL, /* gmst_ubak, gmst_disco, gmst_mvitals */ /* pline.c */ UNDEFINED_PTR, /* gamelog */ /* region.c */ diff --git a/src/nhlua.c b/src/nhlua.c index 6dfeaea26..f0238bdc5 100644 --- a/src/nhlua.c +++ b/src/nhlua.c @@ -1572,74 +1572,71 @@ nhl_callback(lua_State *L) staticfn int nhl_gamestate(lua_State *L) { - static long gmst_moves = 0; - static struct obj *gmst_invent = NULL; - static genericptr_t - *gmst_ubak = NULL, *gmst_disco = NULL, *gmst_mvitals= NULL; - static boolean gmst_stored = FALSE; long wornmask; struct obj *otmp; int argc = lua_gettop(L); boolean reststate = (argc > 0) ? lua_toboolean(L, -1) : FALSE; debugpline4("gamestate: %d:%d (%c vs %c)", u.uz.dnum, u.uz.dlevel, - reststate ? 'T' : 'F', gmst_stored ? 't' : 'f'); + reststate ? 'T' : 'F', gg.gmst_stored ? 't' : 'f'); - if (reststate && gmst_stored) { + if (reststate && gg.gmst_stored) { d_level cur_uz = u.uz, cur_uz0 = u.uz0; /* restore game state */ - pline("Resetting time to move #%ld.", gmst_moves); - gm.moves = gmst_moves; + gm.moves = gg.gmst_moves; + pline("Resetting time to move #%ld.", gm.moves); + gg.gmst_moves = 0L; + gl.lastinvnr = 51; while (gi.invent) useupall(gi.invent); - while ((otmp = gmst_invent) != NULL) { + while ((otmp = gg.gmst_invent) != NULL) { wornmask = otmp->owornmask; otmp->owornmask = 0L; - extract_nobj(otmp, &gmst_invent); + extract_nobj(otmp, &gg.gmst_invent); addinv_nomerge(otmp); if (wornmask) setworn(otmp, wornmask); } - assert(gmst_ubak != NULL); - (void) memcpy((genericptr_t) &u, gmst_ubak, sizeof u); - free(gmst_ubak), gmst_ubak = NULL; - assert(gmst_disco != NULL); - (void) memcpy((genericptr_t) &gd.disco, gmst_disco, sizeof gd.disco); - free(gmst_disco), gmst_disco = NULL; - assert(gmst_mvitals != NULL); - (void) memcpy((genericptr_t) &gm.mvitals, gmst_mvitals, + assert(gg.gmst_ubak != NULL); + (void) memcpy((genericptr_t) &u, gg.gmst_ubak, sizeof u); + assert(gg.gmst_disco != NULL); + (void) memcpy((genericptr_t) &gd.disco, gg.gmst_disco, + sizeof gd.disco); + assert(gg.gmst_mvitals != NULL); + (void) memcpy((genericptr_t) &gm.mvitals, gg.gmst_mvitals, sizeof gm.mvitals); - free(gmst_mvitals), gmst_mvitals = NULL; /* some restored state would confuse the level change in progress */ u.uz = cur_uz, u.uz0 = cur_uz0; init_uhunger(); - gmst_stored = FALSE; - } else if (!reststate && !gmst_stored) { + free_tutorial(); /* release gg.gmst_XYZ */ + gg.gmst_stored = FALSE; + } else if (!reststate && !gg.gmst_stored) { /* store game state */ + gg.gmst_moves = gm.moves; while ((otmp = gi.invent) != NULL) { wornmask = otmp->owornmask; setnotworn(otmp); freeinv(otmp); - otmp->nobj = gmst_invent; - otmp->owornmask = wornmask; - gmst_invent = otmp; + otmp->owornmask = wornmask; /* flag for later restore */ + otmp->nobj = gg.gmst_invent; + gg.gmst_invent = otmp; } gl.lastinvnr = 51; /* next inv letter to try to use will be 'a' */ - gmst_moves = gm.moves; - gmst_ubak = (genericptr_t) alloc(sizeof u); - (void) memcpy(gmst_ubak, (genericptr_t) &u, sizeof u); - gmst_disco = (genericptr_t) alloc(sizeof gd.disco); - (void) memcpy(gmst_disco, (genericptr_t) &gd.disco, sizeof gd.disco); - gmst_mvitals = (genericptr_t) alloc(sizeof gm.mvitals); - (void) memcpy(gmst_mvitals, (genericptr_t) &gm.mvitals, + gg.gmst_ubak = (genericptr_t) alloc(sizeof u); + (void) memcpy(gg.gmst_ubak, (genericptr_t) &u, sizeof u); + gg.gmst_disco = (genericptr_t) alloc(sizeof gd.disco); + (void) memcpy(gg.gmst_disco, (genericptr_t) &gd.disco, + sizeof gd.disco); + gg.gmst_mvitals = (genericptr_t) alloc(sizeof gm.mvitals); + (void) memcpy(gg.gmst_mvitals, (genericptr_t) &gm.mvitals, sizeof gm.mvitals); - gmst_stored = TRUE; + gg.gmst_stored = TRUE; } else { impossible("nhl_gamestate: inconsistent state (%s vs %s)", reststate ? "restore" : "save", - gmst_stored ? "already stored" : "not stored"); + gg.gmst_stored ? "already stored" : "not stored"); } update_inventory(); return 0; @@ -1647,6 +1644,35 @@ nhl_gamestate(lua_State *L) RESTORE_WARNING_UNREACHABLE_CODE +/* free dynamic date allocated when entering tutorial; + called when exiting tutorial normally or if player quits while in it */ +void +free_tutorial(void) +{ + struct obj *otmp; + + /* for normal tutorial exit, gmst_invent will already be Null */ + while ((otmp = gg.gmst_invent) != 0) { + /* set otmp->where = OBJ_FREE, otmp->nobj = NULL */ + extract_nobj(otmp, &gg.gmst_invent); + /* gmst_invent is a list of invent items sequestered when entering + the tutorial; for them, owornmask is used as a flag to re-wear + them when exiting tutorial, not that they are currently worn; + clear it to avoid a "deleting worn obj" complaint from obfree() */ + otmp->owornmask = 0L; + /* dealloc_obj() isn't enough (for containers, it assumes that + caller has already freed their contents) */ + obfree(otmp, (struct obj *) 0); + } + + if (gg.gmst_ubak) + free(gg.gmst_ubak), gg.gmst_ubak = NULL; + if (gg.gmst_disco) + free(gg.gmst_disco), gg.gmst_disco = NULL; + if (gg.gmst_mvitals) + free(gg.gmst_mvitals), gg.gmst_mvitals = NULL; +} + /* called from gotolevel(do.c) */ void tutorial(boolean entering) @@ -2027,7 +2053,8 @@ nhl_loadlua(lua_State *L, const char *fname) * in use, and fseek(SEEK_END) only yields an upper bound on * the actual amount of data in that situation.] */ - if ((cnt = dlb_fread(bufin, 1, min((int) buflen, LOADCHUNKSIZE), fh)) < 0L) + if ((cnt = dlb_fread(bufin, 1, min((int) buflen, LOADCHUNKSIZE), fh)) + < 0L) break; buflen -= cnt; /* set up for next iteration, if any */ if (cnt == 0L) { diff --git a/src/save.c b/src/save.c index f5fe3e496..789b3d68d 100644 --- a/src/save.c +++ b/src/save.c @@ -1210,6 +1210,7 @@ freedynamicdata(void) freeroleoptvals(); /* saveoptvals(&tnhfp) */ cmdq_clear(CQ_CANNED); cmdq_clear(CQ_REPEAT); + free_tutorial(); /* (only needed if quiting while in tutorial) */ /* some pointers in iflags */ if (iflags.wc_font_map)