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.

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
 

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 */
 

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);
 }
 
 

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 */

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()
 {

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);
 }
 

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*/

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;

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;
 }
 

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;
 }

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);