From: PatR Date: Mon, 25 Mar 2019 00:50:26 +0000 (-0700) Subject: curses message recall, memory leaks X-Git-Tag: NetHack-3.6.2_Released~33^2~1 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=ee53a9fea6be8fe4c3173784bea2a129a0fdd447;p=nethack curses message recall, memory leaks Using ^P right after resize or 'O' of align_message, align_status, statuslines, or windowborders would result in 'curses_display_nhmenu: attempt to display empty menu' because some memory cleanup I added several weeks back was being executed when the curses interface tore down and recreated its internal windows. This fixes ^P handling by making sure that that menu (which is just text but uses a menu to support '>'/'<'/'^'/'|' scrolling) will never be empty and it also fixes the window deletion to not throw away message history until it's final deletion at exit time. ^P uses a popup window to display previous messages and it was never deleting that window, just creating a new one each time. Same with the routine which displays an external help file. Using either or combination of both close to 5000 times would probably make internal window creation get stuck in an infinite loop. Delete those windows after they're used so it'll never be put to the test. The memory cleanup I added for map/status/messages/invent was only being preformed at end of game, not when saving. Fix that too. --- diff --git a/doc/fixes36.2 b/doc/fixes36.2 index 9c996dbdf..405f5bba6 100644 --- a/doc/fixes36.2 +++ b/doc/fixes36.2 @@ -1,4 +1,4 @@ -$NHDT-Branch: NetHack-3.6.2-beta01 $:$NHDT-Revision: 1.280 $ $NHDT-Date: 1553387147 2019/03/24 00:25:47 $ +$NHDT-Branch: NetHack-3.6.2-beta01 $:$NHDT-Revision: 1.281 $ $NHDT-Date: 1553475022 2019/03/25 00:50:22 $ This fixes36.2 file is here to capture information about updates in the 3.6.x lineage following the release of 3.6.1 in April 2018. Please note, however, @@ -426,7 +426,7 @@ fix "placing monster over another?" warning for vault guards status highlighting classifies gold, time, and experience-points as data type 'long' but when selecting hilite rule to use treated them as 'int' a config file line with OPTIONS=symset:default, roguesymset:RogueEpyx - was disabling color on RogueEpyx even though the symset is + was disabling color on RogueEpyx even though the symset is meant to have color; that was due to an errant init_symbols() call during the processing of symset:default done after RogueEpyx had already been processed @@ -494,6 +494,15 @@ curses: 'hitpointbar' now works even when status highlighting is disabled curses: add 'statuslines' option (value is 2 or 3) curses: change 'windowborders' option's value (0=Off, 1=On, or 2=Auto instead of 1=On, 2=Off, 3=Auto; can now be changed via 'O' during play) +curses: free allocated memory prior to exit; previous attempt at this only + handled game-over, not saving +curses: the memory deallocation was releasing previous messages when curses + interface reinitialized its main windows after resize or 'O' was used + to set align_message, align_status, statuslines, or windowborders; + using ^P before next new message gave a warning about empty menu +curses: popup window to show ^P output was removed from screen but never + deleted; further ^P's repeated that cycle; likewise for help which + displays an external text file vms: add compile of isaac64.c to Makefile.src and vmsbuild.com vms+curses: add compile support but it is known to fail to build diff --git a/include/wincurs.h b/include/wincurs.h index d4db5518b..79893ddcd 100644 --- a/include/wincurs.h +++ b/include/wincurs.h @@ -128,6 +128,7 @@ extern void curses_refresh_nhwin(winid wid); extern void curses_refresh_nethack_windows(void); extern void curses_del_nhwin(winid wid); extern void curses_del_wid(winid wid); +extern void curs_destroy_all_wins(void); extern void curses_putch(winid wid, int x, int y, int ch, int color, int attrs); extern void curses_get_window_size(winid wid, int *height, int *width); @@ -184,7 +185,7 @@ extern void curses_add_nhmenu_item(winid wid, int glyph, extern void curses_finalize_nhmenu(winid wid, const char *prompt); extern int curses_display_nhmenu(winid wid, int how, MENU_ITEM_P **_selected); extern boolean curses_menu_exists(winid wid); -extern void curses_del_menu(winid wid); +extern void curses_del_menu(winid, boolean); /* cursstat.c */ diff --git a/win/curses/cursdial.c b/win/curses/cursdial.c index 7e76e046c..af337f858 100644 --- a/win/curses/cursdial.c +++ b/win/curses/cursdial.c @@ -498,16 +498,21 @@ curses_create_nhmenu(winid wid) while (menu_item_ptr->next_item != NULL) { tmp_menu_item = menu_item_ptr->next_item; free((genericptr_t) menu_item_ptr->str); - free(menu_item_ptr); + free((genericptr_t) menu_item_ptr); menu_item_ptr = tmp_menu_item; } free((genericptr_t) menu_item_ptr->str); - free(menu_item_ptr); /* Last entry */ + free((genericptr_t) menu_item_ptr); /* Last entry */ new_menu->entries = NULL; } if (new_menu->prompt != NULL) { /* Reusing existing menu */ free((genericptr_t) new_menu->prompt); + new_menu->prompt = NULL; } + new_menu->num_pages = 0; + new_menu->height = 0; + new_menu->width = 0; + new_menu->reuse_accels = FALSE; return; } @@ -708,7 +713,7 @@ curses_menu_exists(winid wid) /* Delete the menu associated with the given NetHack winid from memory */ void -curses_del_menu(winid wid) +curses_del_menu(winid wid, boolean del_wid_too) { nhmenu_item *tmp_menu_item; nhmenu_item *menu_item_ptr; @@ -735,21 +740,21 @@ curses_del_menu(winid wid) } /* Now unlink the menu from the list and free it as well */ - if (current_menu->prev_menu != NULL) { - tmpmenu = current_menu->prev_menu; + if ((tmpmenu = current_menu->prev_menu) != NULL) { tmpmenu->next_menu = current_menu->next_menu; } else { nhmenus = current_menu->next_menu; /* New head mode or NULL */ } - if (current_menu->next_menu != NULL) { - tmpmenu = current_menu->next_menu; + if ((tmpmenu = current_menu->next_menu) != NULL) { tmpmenu->prev_menu = current_menu->prev_menu; } - free((genericptr_t) current_menu->prompt); - free(current_menu); + if (current_menu->prompt) + free((genericptr_t) current_menu->prompt); + free((genericptr_t) current_menu); - curses_del_wid(wid); + if (del_wid_too) + curses_del_wid(wid); } diff --git a/win/curses/cursdial.h b/win/curses/cursdial.h index 9c15a0775..0e12be6a5 100644 --- a/win/curses/cursdial.h +++ b/win/curses/cursdial.h @@ -13,14 +13,12 @@ int curses_character_input_dialog(const char *prompt, const char *choices, CHAR_P def); int curses_ext_cmd(void); void curses_create_nhmenu(winid wid); -void curses_add_nhmenu_item(winid wid, int glyph, const ANY_P * identifier, +void curses_add_nhmenu_item(winid wid, int glyph, const ANY_P *identifier, CHAR_P accelerator, CHAR_P group_accel, int attr, const char *str, BOOLEAN_P presel); void curses_finalize_nhmenu(winid wid, const char *prompt); -int curses_display_nhmenu(winid wid, int how, MENU_ITEM_P ** _selected); +int curses_display_nhmenu(winid wid, int how, MENU_ITEM_P **_selected); boolean curses_menu_exists(winid wid); -void curses_del_menu(winid wid); - - +void curses_del_menu(winid, boolean); #endif /* CURSDIAL_H */ diff --git a/win/curses/cursinit.c b/win/curses/cursinit.c index 2434f676b..924f41c8d 100644 --- a/win/curses/cursinit.c +++ b/win/curses/cursinit.c @@ -107,7 +107,7 @@ set_window_position(int *winx, int *winy, int *winw, int *winh, } } -/* Create the "main" nonvolitile windows used by nethack */ +/* Create the "main" nonvolatile windows used by nethack */ void curses_create_main_windows() { diff --git a/win/curses/cursmain.c b/win/curses/cursmain.c index 5cc30c126..100719370 100644 --- a/win/curses/cursmain.c +++ b/win/curses/cursmain.c @@ -241,6 +241,12 @@ curses_get_nh_event() void curses_exit_nhwindows(const char *str) { + curses_destroy_nhwindow(INV_WIN); + curses_destroy_nhwindow(MAP_WIN); + curses_destroy_nhwindow(STATUS_WIN); + curses_destroy_nhwindow(MESSAGE_WIN); + curs_destroy_all_wins(); + curses_cleanup(); curs_set(orig_cursor); endwin(); @@ -340,6 +346,21 @@ curses_display_nhwindow(winid wid, BOOLEAN_P block) void curses_destroy_nhwindow(winid wid) { + switch (wid) { + case MESSAGE_WIN: + curses_teardown_messages(); /* discard ^P message history data */ + break; + case STATUS_WIN: + curses_status_finish(); /* discard cached status data */ + break; + case INV_WIN: + iflags.perm_invent = 0; /* avoid unexpected update_inventory() */ + break; + case MAP_WIN: + break; + default: + break; + } curses_del_nhwin(wid); } diff --git a/win/curses/cursmesg.c b/win/curses/cursmesg.c index 55cb157f9..2323be6cf 100644 --- a/win/curses/cursmesg.c +++ b/win/curses/cursmesg.c @@ -148,9 +148,9 @@ curses_message_win_puts(const char *message, boolean recursed) int -curses_block(boolean noscroll) -/* noscroll - blocking because of msgtype = stop/alert */ -/* else blocking because window is full, so need to scroll after */ +curses_block(boolean noscroll) /* noscroll - blocking because of msgtype + * = stop/alert else blocking because + * window is full, so need to scroll after */ { int height, width, ret = 0; WINDOW *win = curses_get_nhwin(MESSAGE_WIN); @@ -165,7 +165,7 @@ curses_block(boolean noscroll) * just any key, as we want to prevent YASD from * riding direction keys. */ while ((ret = wgetch(win)) != 0 && !index(resp, (char) ret)) - continue; + continue; if (height == 1) { curses_clear_unhighlight_message_window(); } else { @@ -233,13 +233,10 @@ curses_last_messages() nhprev_mesg *mesg; int i; - if (border) { - mx = 1; - my = 1; - } else { - mx = 0; - my = 0; - } + if (border) + mx = my = 1; + else + mx = my = 0; for (i = (num_messages - 1); i > 0; i--) { mesg = get_msg_line(TRUE, i); @@ -306,9 +303,13 @@ curses_prev_mesg() curses_add_menu(wid, NO_GLYPH, &Id, 0, 0, A_NORMAL, mesg->str, FALSE); turn = mesg->turn; } + if (!count) + curses_add_menu(wid, NO_GLYPH, &Id, 0, 0, A_NORMAL, + "[No past messages available.]", FALSE); curses_end_menu(wid, ""); curses_select_menu(wid, PICK_NONE, &selected); + curses_del_wid(wid); } @@ -655,20 +656,19 @@ get_msg_line(boolean reverse, int mindex) if (reverse) { current_mesg = last_mesg; for (count = 0; count < mindex; count++) { - if (current_mesg == NULL) { - return NULL; - } + if (!current_mesg) + break; current_mesg = current_mesg->prev_mesg; } - return current_mesg; } else { current_mesg = first_mesg; for (count = 0; count < mindex; count++) { - if (current_mesg == NULL) { - return NULL; - } + if (!current_mesg) + break; current_mesg = current_mesg->next_mesg; } - return current_mesg; } + return current_mesg; } + +/*cursmesg.c*/ diff --git a/win/curses/cursmisc.c b/win/curses/cursmisc.c index b11083b0c..fd712fac6 100644 --- a/win/curses/cursmisc.c +++ b/win/curses/cursmisc.c @@ -177,9 +177,9 @@ curses_bail(const char *mesg) winid curses_get_wid(int type) { - winid ret; static winid menu_wid = 20; /* Always even */ static winid text_wid = 21; /* Always odd */ + winid ret; switch (type) { case NHW_MESSAGE: @@ -586,11 +586,9 @@ curses_view_file(const char *filename, boolean must_exist) menu_item *selected = NULL; dlb *fp = dlb_fopen(filename, "r"); - if ((fp == NULL) && (must_exist)) { - pline("Cannot open %s for reading!", filename); - } - if (fp == NULL) { + if (must_exist) + pline("Cannot open \"%s\" for reading!", filename); return; } @@ -605,6 +603,7 @@ curses_view_file(const char *filename, boolean must_exist) dlb_fclose(fp); curses_end_menu(wid, ""); curses_select_menu(wid, PICK_NONE, &selected); + curses_del_wid(wid); } @@ -675,6 +674,9 @@ curses_convert_attr(int attr) case ATR_BOLD: curses_attr = A_BOLD; break; + case ATR_DIM: + curses_attr = A_DIM; + break; case ATR_BLINK: curses_attr = A_BLINK; break; diff --git a/win/curses/cursstat.c b/win/curses/cursstat.c index d8dfeeb5f..9dfd39fe8 100644 --- a/win/curses/cursstat.c +++ b/win/curses/cursstat.c @@ -66,6 +66,7 @@ curses_status_init() /* let genl_status_init do most of the initialization */ genl_status_init(); + return; } void @@ -78,8 +79,9 @@ curses_status_finish() if (status_vals_long[i]) free(status_vals_long[i]), status_vals_long[i] = (char *) 0; } - genl_status_finish(); #endif /* STATUS_HILITES */ + + genl_status_finish(); return; } diff --git a/win/curses/curswins.c b/win/curses/curswins.c index 420612ae6..ae28140b9 100644 --- a/win/curses/curswins.c +++ b/win/curses/curswins.c @@ -147,7 +147,7 @@ curses_create_window(int width, int height, orient orientation) /* Erase and delete curses window, and refresh standard windows */ void -curses_destroy_win(WINDOW * win) +curses_destroy_win(WINDOW *win) { werase(win); wrefresh(win); @@ -303,8 +303,11 @@ void curses_del_nhwin(winid wid) { if (curses_is_menu(wid) || curses_is_text(wid)) { - curses_del_menu(wid); + curses_del_menu(wid, TRUE); return; + } else if (wid == INV_WIN) { + curses_del_menu(wid, TRUE); + /* don't return yet */ } if (!is_main_window(wid)) { @@ -312,9 +315,6 @@ curses_del_nhwin(winid wid) wid); return; } - if (wid == MESSAGE_WIN) { - curses_teardown_messages(); - } nhwins[wid].curwin = NULL; nhwins[wid].nhwin = -1; } @@ -326,31 +326,35 @@ void curses_del_wid(winid wid) { nethack_wid *tmpwid; - nethack_wid *widptr = nhwids; + nethack_wid *widptr; if (curses_is_menu(wid) || curses_is_text(wid)) { - curses_del_menu(wid); + curses_del_menu(wid, FALSE); } - while (widptr != NULL) { + for (widptr = nhwids; widptr; widptr = widptr->next_wid) { if (widptr->nhwid == wid) { - if (widptr->prev_wid != NULL) { - tmpwid = widptr->prev_wid; + if ((tmpwid = widptr->prev_wid) != NULL) { tmpwid->next_wid = widptr->next_wid; } else { nhwids = widptr->next_wid; /* New head mode, or NULL */ } - if (widptr->next_wid != NULL) { - tmpwid = widptr->next_wid; + if ((tmpwid = widptr->next_wid) != NULL) { tmpwid->prev_wid = widptr->prev_wid; } free(widptr); break; } - widptr = widptr->next_wid; } } +/* called by destroy_nhwindows() prior to exit */ +void +curs_destroy_all_wins() +{ + while (nhwids) + curses_del_wid(nhwids->nhwid); +} /* Print a single character in the given window at the given coordinates */ @@ -439,15 +443,11 @@ curses_window_has_border(winid wid) boolean curses_window_exists(winid wid) { - nethack_wid *widptr = nhwids; + nethack_wid *widptr; - while (widptr != NULL) { - if (widptr->nhwid == wid) { + for (widptr = nhwids; widptr; widptr = widptr->next_wid) + if (widptr->nhwid == wid) return TRUE; - } - - widptr = widptr->next_wid; - } return FALSE; } diff --git a/win/curses/curswins.h b/win/curses/curswins.h index dfa7b7a1e..ad2e5902f 100644 --- a/win/curses/curswins.h +++ b/win/curses/curswins.h @@ -20,6 +20,7 @@ void curses_add_wid(winid wid); void curses_refresh_nhwin(winid wid); void curses_del_nhwin(winid wid); void curses_del_wid(winid wid); +void curs_destroy_all_wins(void); void curses_putch(winid wid, int x, int y, int ch, int color, int attrs); void curses_get_window_xy(winid wid, int *x, int *y); boolean curses_window_has_border(winid wid);