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

fix memory leak in iter_mons_safe()

Browse Source 
While hunting for a memory leak in object allocation--which I haven't
found yet--I discovered one in monster movement.  iter_mons_safe()
allocates an array of (monst *) pointers for the monsters on the
current level, loops over that array to call a function for each
one, then frees the array.  But if the game ends while that called
function is running, execution never returns to iter_mons_safe() so
it wasn't able to free the memory.

Since that can happen at most once per game, it wasn't a signifcant
leak.  This fixes it anyway.

There was a second issue:  make sure that iter_mons_safe() doesn't
call alloc(0) to make the temporary array for zero monsters when
there aren't any on the level.  That might not be able to happen for
monster movement but the routine is written to be more general than
just movement.  alloc(0) could confuse the MONITOR_HEAP code.  In
C89/C90 I think malloc(0) is allowed to return NULL (don't recall
for sure; maybe that was just known pre-standard behavior for some
implementations).  Null return would trigger a panic even without
MONITOR_HEAP.  Don't know about C99 and later.
This commit is contained in:
PatR
2024-01-23 23:04:06 -08:00
parent 1d165f20ee
commit 78252de3bc
4 changed files with 53 additions and 36 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
include/decl.h
View File

@@ -1,4 +1,4 @@
/* NetHack 3.7 decl.h $NHDT-Date: 1704043695 2023/12/31 17:28:15 $ $NHDT-Branch: keni-luabits2 $:$NHDT-Revision: 1.351 $ */
/* NetHack 3.7 decl.h $NHDT-Date: 1706079834 2024/01/24 07:03:54 $ $NHDT-Branch: NetHack-3.7 $:$NHDT-Revision: 1.355 $ */
/* Copyright (c) Stichting Mathematisch Centrum, Amsterdam, 1985. */
/*-Copyright (c) Michael Allison, 2007. */
/* NetHack may be freely redistributed. See license for details. */
@@ -473,6 +473,10 @@ struct instance_globals_i {
unsigned invbufsiz;
int in_sync_perminvent;
/* mon.c */
struct monst **itermonarr; /* temporary array of all N monsters
* on the current level */
/* restore.c */
struct bucket *id_map;

4
src/decl.c
View File

@@ -1,4 +1,4 @@
/* NetHack 3.7 decl.c $NHDT-Date: 1704043695 2023/12/31 17:28:15 $ $NHDT-Branch: keni-luabits2 $:$NHDT-Revision: 1.309 $ */
/* NetHack 3.7 decl.c $NHDT-Date: 1706079841 2024/01/24 07:04:01 $ $NHDT-Branch: NetHack-3.7 $:$NHDT-Revision: 1.314 $ */
/* Copyright (c) Stichting Mathematisch Centrum, Amsterdam, 1985. */
/*-Copyright (c) Michael Allison, 2009. */
/* NetHack may be freely redistributed. See license for details. */
@@ -445,6 +445,8 @@ const struct instance_globals_i g_init_i = {
NULL, /* invbuf */
0U, /* invbufsize */
0, /* in_sync_perminvent */
/* mon.c */
NULL, /* itermonarr */
/* restore.c */
UNDEFINED_PTR, /* id_map */
/* sp_lev.c */

75
src/mon.c
View File

@@ -1,4 +1,4 @@
/* NetHack 3.7 mon.c $NHDT-Date: 1702023272 2023/12/08 08:14:32 $ $NHDT-Branch: NetHack-3.7 $:$NHDT-Revision: 1.532 $ */
/* NetHack 3.7 mon.c $NHDT-Date: 1706079843 2024/01/24 07:04:03 $ $NHDT-Branch: NetHack-3.7 $:$NHDT-Revision: 1.549 $ */
/* Copyright (c) Stichting Mathematisch Centrum, Amsterdam, 1985. */
/*-Copyright (c) Derek S. Ray, 2015. */
/* NetHack may be freely redistributed. See license for details. */
@@ -4102,38 +4102,45 @@ normal_shape(struct monst *mon)
}
}
/* iterate all monsters on the level, even dead or off-map ones,
calling func for each monster. if func returns TRUE, stop iterating.
safe for list deletions and insertions, and guarantees
calling func once per monster in the list. */
void
iter_mons_safe(boolean (*func)(struct monst *))
{
struct monst **monarr;
int i = 0, nmons = 0;
struct monst *mtmp;
boolean stopiter = FALSE;
/* Iterate all monsters on the level, even dead or off-map ones, calling
bfunc() for each monster. If bfunc() returns TRUE, stop iterating.
If the game ends during the call to bfunc(), then freedynamicdata()
will need to free 'monarr' for us so we make a copy of it in struct g.
for (mtmp = fmon; mtmp; mtmp = mtmp->nmon)
Safe for list deletions and insertions, and guarantees calling bfunc()
once per monster in fmon unless it returns TRUE (or game ends). */
void
iter_mons_safe(boolean (*bfunc)(struct monst *))
{
struct monst **monarr, *mtmp;
int i, nmons;
for (nmons = 0, mtmp = fmon; mtmp; mtmp = mtmp->nmon)
nmons++;
monarr = (struct monst **) alloc(nmons * sizeof(struct monst *));
for (mtmp = fmon; mtmp; mtmp = mtmp->nmon)
monarr[i++] = mtmp;
if (nmons) {
monarr = (struct monst **) alloc(nmons * sizeof (struct monst *));
gi.itermonarr = monarr; /* accessible to freedynamicdata() */
for (i = 0; i < nmons && !stopiter; i++) {
mtmp = monarr[i];
if (func(mtmp))
stopiter = TRUE;
for (i = 0, mtmp = fmon; mtmp; mtmp = mtmp->nmon)
monarr[i++] = mtmp;
for (i = 0; i < nmons; i++) {
mtmp = monarr[i];
if ((*bfunc)(mtmp))
break;
}
free(monarr);
gi.itermonarr = NULL;
}
free(monarr);
return;
}
/* iterate all living monsters on current level, calling func for each. */
void
iter_mons(void (*func)(struct monst *))
iter_mons(void (*vfunc)(struct monst *))
{
struct monst *mtmp, *mtmp2;
@@ -4141,15 +4148,16 @@ iter_mons(void (*func)(struct monst *))
mtmp2 = mtmp->nmon;
if (DEADMONSTER(mtmp) || mon_offmap(mtmp))
continue;
func(mtmp);
(*vfunc)(mtmp);
}
return;
}
/* iterate all living monsters on current level, calling func for each.
if func returns TRUE, stop and return that monster. */
struct monst *
get_iter_mons(boolean (*func)(struct monst *))
get_iter_mons(boolean (*bfunc)(struct monst *))
{
struct monst *mtmp, *mtmp2;
@@ -4157,18 +4165,19 @@ get_iter_mons(boolean (*func)(struct monst *))
mtmp2 = mtmp->nmon;
if (DEADMONSTER(mtmp) || mon_offmap(mtmp))
continue;
if (func(mtmp))
return mtmp;
if ((*bfunc)(mtmp))
break;
}
return (struct monst *) 0;
return mtmp;
}
/* iterate all living monsters on current level, calling func for each,
passing x,y to the function.
if func returns TRUE, stop and return that monster. */
struct monst *
get_iter_mons_xy(boolean (*func)(struct monst *, coordxy, coordxy),
coordxy x, coordxy y)
get_iter_mons_xy(
boolean (*bfunc)(struct monst *, coordxy, coordxy),
coordxy x, coordxy y)
{
struct monst *mtmp, *mtmp2;
@@ -4176,10 +4185,10 @@ get_iter_mons_xy(boolean (*func)(struct monst *, coordxy, coordxy),
mtmp2 = mtmp->nmon;
if (DEADMONSTER(mtmp) || mon_offmap(mtmp))
continue;
if (func(mtmp, x, y))
return mtmp;
if ((*bfunc)(mtmp, x, y))
break;
}
return (struct monst *) 0;
return mtmp;
}

4
src/save.c
View File

@@ -1,4 +1,4 @@
/* NetHack 3.7 save.c $NHDT-Date: 1689629246 2023/07/17 21:27:26 $ $NHDT-Branch: NetHack-3.7 $:$NHDT-Revision: 1.207 $ */
/* NetHack 3.7 save.c $NHDT-Date: 1706079844 2024/01/24 07:04:04 $ $NHDT-Branch: NetHack-3.7 $:$NHDT-Revision: 1.214 $ */
/* Copyright (c) Stichting Mathematisch Centrum, Amsterdam, 1985. */
/*-Copyright (c) Michael Allison, 2009. */
/* NetHack may be freely redistributed. See license for details. */
@@ -1184,6 +1184,8 @@ freedynamicdata(void)
/* move-specific data */
dmonsfree(); /* release dead monsters */
if (gi.itermonarr)
free((genericptr_t) gi.itermonarr), gi.itermonarr = NULL;
/* level-specific data */
done_object_cleanup(); /* maybe force some OBJ_FREE items onto map */