minor memory leak

I ran the fuzzer with MONITOR_HEAP enabled and heaputil found a dozen
or so un-free'd allocations, all made by the same dupstr() call in
special_handling() for "symset" and "roguesymset".  (Reproducible with
a few tens of thousands of fuzzer moves, although you have to take
over from the fuzzer and make a clean exit rather than just interrupt
it or there'll be lots of other un-free'd memory.)  I haven't actually
figured out how/why it was leaking, but reorganizing the code has made
the leak go away (according to a couple of even longer fuzzer runs) so
I'm settling for that.
This commit is contained in:
PatR
2018-12-29 20:41:16 -08:00
parent 39b6f7f462
commit adf64764f4
2 changed files with 46 additions and 89 deletions
+9 -20
View File
@@ -1,4 +1,4 @@
/* NetHack 3.6 files.c $NHDT-Date: 1545702598 2018/12/25 01:49:58 $ $NHDT-Branch: NetHack-3.6.2-beta01 $:$NHDT-Revision: 1.248 $ */ /* NetHack 3.6 files.c $NHDT-Date: 1546144856 2018/12/30 04:40:56 $ $NHDT-Branch: NetHack-3.6.2-beta01 $:$NHDT-Revision: 1.249 $ */
/* Copyright (c) Stichting Mathematisch Centrum, Amsterdam, 1985. */ /* Copyright (c) Stichting Mathematisch Centrum, Amsterdam, 1985. */
/*-Copyright (c) Derek S. Ray, 2015. */ /*-Copyright (c) Derek S. Ray, 2015. */
/* NetHack may be freely redistributed. See license for details. */ /* NetHack may be freely redistributed. See license for details. */
@@ -3264,37 +3264,26 @@ int which_set;
switch (symp->idx) { switch (symp->idx) {
case 0: case 0:
tmpsp = tmpsp = (struct symsetentry *) alloc(sizeof *tmpsp);
(struct symsetentry *) alloc(sizeof (struct symsetentry)); tmpsp->next = symset_list;
tmpsp->next = (struct symsetentry *) 0; symset_list = tmpsp;
if (!symset_list) { tmpsp->idx = symset_count++;
symset_list = tmpsp;
symset_count = 0;
} else {
symset_count++;
tmpsp->next = symset_list;
symset_list = tmpsp;
}
tmpsp->idx = symset_count;
tmpsp->name = dupstr(bufp); tmpsp->name = dupstr(bufp);
tmpsp->desc = (char *) 0; tmpsp->desc = (char *) 0;
tmpsp->nocolor = 0; tmpsp->handling = H_UNK;
/* initialize restriction bits */ /* initialize restriction bits */
tmpsp->nocolor = 0;
tmpsp->primary = 0; tmpsp->primary = 0;
tmpsp->rogue = 0; tmpsp->rogue = 0;
break; break;
case 2: case 2:
/* handler type identified */ /* handler type identified */
tmpsp = symset_list; /* most recent symset */ tmpsp = symset_list; /* most recent symset */
tmpsp->handling = H_UNK; for (i = 0; known_handling[i]; ++i)
i = 0;
while (known_handling[i]) {
if (!strcmpi(known_handling[i], bufp)) { if (!strcmpi(known_handling[i], bufp)) {
tmpsp->handling = i; tmpsp->handling = i;
break; /* while loop */ break; /* for loop */
} }
i++;
}
break; break;
case 3: /* description:something */ case 3: /* description:something */
tmpsp = symset_list; /* most recent symset */ tmpsp = symset_list; /* most recent symset */
+37 -69
View File
@@ -1,4 +1,4 @@
/* NetHack 3.6 options.c $NHDT-Date: 1544773907 2018/12/14 07:51:47 $ $NHDT-Branch: NetHack-3.6.2-beta01 $:$NHDT-Revision: 1.347 $ */ /* NetHack 3.6 options.c $NHDT-Date: 1546144857 2018/12/30 04:40:57 $ $NHDT-Branch: NetHack-3.6.2-beta01 $:$NHDT-Revision: 1.349 $ */
/* Copyright (c) Stichting Mathematisch Centrum, Amsterdam, 1985. */ /* Copyright (c) Stichting Mathematisch Centrum, Amsterdam, 1985. */
/*-Copyright (c) Michael Allison, 2008. */ /*-Copyright (c) Michael Allison, 2008. */
/* NetHack may be freely redistributed. See license for details. */ /* NetHack may be freely redistributed. See license for details. */
@@ -5201,8 +5201,7 @@ boolean setinitial, setfromfile;
} else if (!strcmp("symset", optname) } else if (!strcmp("symset", optname)
|| !strcmp("roguesymset", optname)) { || !strcmp("roguesymset", optname)) {
menu_item *symset_pick = (menu_item *) 0; menu_item *symset_pick = (menu_item *) 0;
boolean primaryflag = (*optname == 's'), boolean rogueflag = (*optname == 'r'),
rogueflag = (*optname == 'r'),
ready_to_switch = FALSE, ready_to_switch = FALSE,
nothing_to_do = FALSE; nothing_to_do = FALSE;
char *symset_name, fmtstr[20]; char *symset_name, fmtstr[20];
@@ -5210,37 +5209,33 @@ boolean setinitial, setfromfile;
int res, which_set, setcount = 0, chosen = -2; int res, which_set, setcount = 0, chosen = -2;
which_set = rogueflag ? ROGUESET : PRIMARY; which_set = rogueflag ? ROGUESET : PRIMARY;
symset_list = (struct symsetentry *) 0;
/* clear symset[].name as a flag to read_sym_file() to build list */ /* clear symset[].name as a flag to read_sym_file() to build list */
symset_name = symset[which_set].name; symset_name = symset[which_set].name;
symset[which_set].name = (char *) 0; symset[which_set].name = (char *) 0;
symset_list = (struct symsetentry *) 0;
res = read_sym_file(which_set); res = read_sym_file(which_set);
if (res && symset_list) {
char symsetchoice[BUFSZ];
int let = 'a', biggest = 0, thissize = 0;
sl = symset_list; /* put symset name back */
while (sl) { symset[which_set].name = symset_name;
if (res && symset_list) {
int thissize, biggest = 0;
for (sl = symset_list; sl; sl = sl->next) {
/* check restrictions */ /* check restrictions */
if ((!rogueflag && sl->rogue) if (rogueflag ? sl->primary : sl->rogue)
|| (!primaryflag && sl->primary)) {
sl = sl->next;
continue; continue;
}
setcount++; setcount++;
/* find biggest name */ /* find biggest name */
if (sl->name) thissize = sl->name ? (int) strlen(sl->name) : 0;
thissize = strlen(sl->name);
if (thissize > biggest) if (thissize > biggest)
biggest = thissize; biggest = thissize;
sl = sl->next;
} }
if (!setcount) { if (!setcount) {
pline("There are no appropriate %ssymbol sets available.", pline("There are no appropriate %s symbol sets available.",
(rogueflag) ? "rogue level " rogueflag ? "rogue level" : "primary");
: (primaryflag) ? "primary " : "");
return TRUE; return TRUE;
} }
@@ -5249,29 +5244,20 @@ boolean setinitial, setfromfile;
start_menu(tmpwin); start_menu(tmpwin);
any = zeroany; any = zeroany;
any.a_int = 1; any.a_int = 1;
add_menu(tmpwin, NO_GLYPH, &any, let++, 0, ATR_NONE, add_menu(tmpwin, NO_GLYPH, &any, 0, 0, ATR_NONE,
"Default Symbols", MENU_UNSELECTED); "Default Symbols", MENU_UNSELECTED);
sl = symset_list; for (sl = symset_list; sl; sl = sl->next) {
while (sl) {
/* check restrictions */ /* check restrictions */
if ((!rogueflag && sl->rogue) if (rogueflag ? sl->primary : sl->rogue)
|| (!primaryflag && sl->primary)) {
sl = sl->next;
continue; continue;
}
if (sl->name) { if (sl->name) {
any.a_int = sl->idx + 2; any.a_int = sl->idx + 2;
Sprintf(symsetchoice, fmtstr, sl->name, Sprintf(buf, fmtstr, sl->name, sl->desc ? sl->desc : "");
sl->desc ? sl->desc : ""); add_menu(tmpwin, NO_GLYPH, &any, 0, 0, ATR_NONE,
add_menu(tmpwin, NO_GLYPH, &any, let, 0, ATR_NONE, buf, MENU_UNSELECTED);
symsetchoice, MENU_UNSELECTED);
if (let == 'z')
let = 'A';
else
let++;
} }
sl = sl->next;
} }
Sprintf(buf, "Select %ssymbol set:", Sprintf(buf, "Select %ssymbol set:",
rogueflag ? "rogue level " : ""); rogueflag ? "rogue level " : "");
@@ -5284,30 +5270,20 @@ boolean setinitial, setfromfile;
if (chosen > -1) { if (chosen > -1) {
/* chose an actual symset name from file */ /* chose an actual symset name from file */
sl = symset_list; for (sl = symset_list; sl; sl = sl->next)
while (sl) { if (sl->idx == chosen)
if (sl->idx == chosen) {
if (symset_name) {
free((genericptr_t) symset_name);
symset_name = (char *) 0;
}
/* free the now stale attributes */
clear_symsetentry(which_set, TRUE);
/* transfer only the name of the symbol set */
symset[which_set].name = dupstr(sl->name);
ready_to_switch = TRUE;
break; break;
} if (sl) {
sl = sl->next; /* free the now stale attributes */
clear_symsetentry(which_set, TRUE);
/* transfer only the name of the symbol set */
symset[which_set].name = dupstr(sl->name);
ready_to_switch = TRUE;
} }
} else if (chosen == -1) { } else if (chosen == -1) {
/* explicit selection of defaults */ /* explicit selection of defaults */
/* free the now stale symset attributes */ /* free the now stale symset attributes */
if (symset_name) {
free((genericptr_t) symset_name);
symset_name = (char *) 0;
}
clear_symsetentry(which_set, TRUE); clear_symsetentry(which_set, TRUE);
} else } else
nothing_to_do = TRUE; nothing_to_do = TRUE;
@@ -5322,26 +5298,18 @@ boolean setinitial, setfromfile;
} }
/* clean up */ /* clean up */
while (symset_list) { while ((sl = symset_list) != 0) {
sl = symset_list;
if (sl->name)
free((genericptr_t) sl->name);
sl->name = (char *) 0;
if (sl->desc)
free((genericptr_t) sl->desc);
sl->desc = (char *) 0;
symset_list = sl->next; symset_list = sl->next;
if (sl->name)
free((genericptr_t) sl->name), sl->name = (char *) 0;
if (sl->desc)
free((genericptr_t) sl->desc), sl->desc = (char *) 0;
free((genericptr_t) sl); free((genericptr_t) sl);
} }
if (nothing_to_do) if (nothing_to_do)
return TRUE; return TRUE;
if (!symset[which_set].name && symset_name)
symset[which_set].name = symset_name; /* not dupstr() here */
/* Set default symbols and clear the handling value */ /* Set default symbols and clear the handling value */
if (rogueflag) if (rogueflag)
init_r_symbols(); init_r_symbols();
@@ -5349,6 +5317,7 @@ boolean setinitial, setfromfile;
init_l_symbols(); init_l_symbols();
if (symset[which_set].name) { if (symset[which_set].name) {
/* non-default symbols */
if (read_sym_file(which_set)) { if (read_sym_file(which_set)) {
ready_to_switch = TRUE; ready_to_switch = TRUE;
} else { } else {
@@ -5367,7 +5336,6 @@ boolean setinitial, setfromfile;
assign_graphics(PRIMARY); assign_graphics(PRIMARY);
preference_update("symset"); preference_update("symset");
need_redraw = TRUE; need_redraw = TRUE;
return TRUE;
} else { } else {
/* didn't match any of the special options */ /* didn't match any of the special options */