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 9c996dbdfed4f284411a4308700f1fa67088632f..405f5bba677a8cd1eedadc7ca3514aabf151b429 100644 (file)
--- 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 d4db5518bc14d66eb51dfeb00506b2a3853ca28a..79893ddcdd8cb5be4bf3bfc817598a65ee26c5d8 100644 (file)
--- 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 7e76e046c5e84fc43cc24332381d8aa484f4966c..af337f8588ede3a83b60062677811d01f2059dba 100644 (file)
--- 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 9c15a077589109998c478dfcfba15ee473b27f1e..0e12be6a55345af352d89eecacad627e3805a43c 100644 (file)
--- 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 2434f676b9e37e39cc1d09a083ea89e005fe71f5..924f41c8d5270b228a4acf8b8b356ccc244aa9fa 100644 (file)
--- 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 5cc30c1268d31f1fa87bf8ddc5e8cb5d6574a504..10071937029e540fc5c4cbad3a35cf889c744f55 100644 (file)
--- 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 55cb157f9da91d7ed9ee5e28b2f535dbe7d82eca..2323be6cf12ee595b8a99a6e6e043815215dbb4b 100644 (file)
--- 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 b11083b0c3dcf542b0362e9b39554149d4931468..fd712fac6e290eb144f8c28446d9b53605ac6fe5 100644 (file)
--- 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 d8dfeeb5fb6d125d8e1a4493dda22129391e65ad..9dfd39fe833fcce08a32fcbec53ca67dcab77f1c 100644 (file)
--- 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 420612ae6fe8fb72abb5a4d1a3c90231dee87398..ae28140b9d1fb7c3c6daaca15d3d27bc66155315 100644 (file)
--- 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 dfa7b7a1ea90c6eab3f6cc16899e95663664c24f..ad2e5902f46fc9282d55983ae2065be4a8aaf287 100644 (file)
--- 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);