date.c vs MONITOR_HEAP

I tried building with MONITOR_HEAP defined for the first time in a
while.  It wasn't pretty.

date.c was calling libc's strdup() instead of our dupstr() so alloc.c
wasn't tracking those allocations when/if MONITOR_HEAP is enabled.
Then it called free() which is actually a call to nhfree() in that
situation.  If the file of allocations and releases was subsequently
fed to heaputil, it would complain about freeing pointers that hadn't
been allocated.

Worse, makedefs and tilemap wouldn't link.  For MONITOR_HEAP,
makedefs was undefining free() in order to avoid nhfree() but it now
links with date.o so got nhfree() calls anyway.  And it wouldn't link
because that routine isn't available without alloc.o.  tilemap
doesn't link with date.o but it does call malloc() and free() and it
wasn't undefining free(), so looked for nhfree() when linking and got
the same no-such-routine failure.

diff --git a/src/date.c b/src/date.c
index 649391f3d8aab118ab819cf6023f7ff6f368d9bc..c6f8acb6919906dd9f1eb68e58dfa49621a7d121 100644 (file)
--- a/src/date.c
+++ b/src/date.c
@@ -1,4 +1,4 @@
-/* NetHack 3.7  date.c  $NHDT-Date: 1608933420 2020/12/25 21:57:00 $  $NHDT-Branch: NetHack-3.7 $:$NHDT-Revision: 1.17 $ */
+/* NetHack 3.7  date.c  $NHDT-Date: 1639622347 2021/12/16 02:39:07 $  $NHDT-Branch: NetHack-3.7 $:$NHDT-Revision: 1.0 $ */
 /* Copyright (c) Michael Allison, 2021.                           */
 /* NetHack may be freely redistributed.  See license for details. */
 
@@ -100,7 +100,7 @@ populate_nomakedefs(struct version_info *version)
             t.tm_sec = atoi(tmpbuf2);
             timeresult = mktime(&t);
             nomakedefs.build_time = (unsigned long) timeresult;
-            nomakedefs.build_date = strdup(tmpbuf1);
+            nomakedefs.build_date = dupstr(tmpbuf1);
         }
 
         nomakedefs.version_number = version->incarnation;
@@ -111,17 +111,16 @@ populate_nomakedefs(struct version_info *version)
         nomakedefs.version_sanity1 = version->entity_count;
         nomakedefs.version_sanity2 = version->struct_sizes1;
         nomakedefs.version_sanity3 = version->struct_sizes2;
-        nomakedefs.version_string = strdup(mdlib_version_string(tmpbuf2, "."));
-        nomakedefs.version_id = strdup(version_id_string(tmpbuf2, sizeof tmpbuf2,
-                                            nomakedefs.build_date));
-        nomakedefs.copyright_banner_c = strdup(bannerc_string(
-                                                tmpbuf2, sizeof tmpbuf2,
-                                                nomakedefs.build_date));
+        nomakedefs.version_string = dupstr(mdlib_version_string(tmpbuf2, "."));
+        nomakedefs.version_id = dupstr(
+           version_id_string(tmpbuf2, sizeof tmpbuf2, nomakedefs.build_date));
+        nomakedefs.copyright_banner_c = dupstr(
+              bannerc_string(tmpbuf2, sizeof tmpbuf2, nomakedefs.build_date));
 #ifdef NETHACK_GIT_SHA
-        nomakedefs.git_sha = strdup(NETHACK_GIT_SHA);
+        nomakedefs.git_sha = dupstr(NETHACK_GIT_SHA);
 #endif
 #ifdef NETHACK_GIT_BRANCH
-        nomakedefs.git_branch = strdup(NETHACK_GIT_BRANCH);
+        nomakedefs.git_branch = dupstr(NETHACK_GIT_BRANCH);
 #endif
 }
 
@@ -129,13 +128,17 @@ void
 free_nomakedefs(void)
 {
     if (nomakedefs.build_date)
-        free((genericptr_t) nomakedefs.build_date);
+        free((genericptr_t) nomakedefs.build_date),
+            nomakedefs.build_date = 0;
     if (nomakedefs.version_string)
-        free((genericptr_t) nomakedefs.version_string);
+        free((genericptr_t) nomakedefs.version_string),
+            nomakedefs.version_string = 0;
     if (nomakedefs.version_id)
-        free((genericptr_t) nomakedefs.version_id);
+        free((genericptr_t) nomakedefs.version_id),
+            nomakedefs.version_id = 0;
     if (nomakedefs.copyright_banner_c)
-        free((genericptr_t) nomakedefs.copyright_banner_c);
+        free((genericptr_t) nomakedefs.copyright_banner_c),
+            nomakedefs.copyright_banner_c = 0;
 }
 
 #endif /* __DATE__ && __TIME__ */

diff --git a/util/makedefs.c b/util/makedefs.c
index af385a07fb1afb4371b8bc384b1b638fb9802b97..db4e5b4971b2e7bfc08d5605c67ca031f92e97ac 100644 (file)
--- a/util/makedefs.c
+++ b/util/makedefs.c
@@ -1,4 +1,4 @@
-/* NetHack 3.7  makedefs.c  $NHDT-Date: 1600855420 2020/09/23 10:03:40 $  $NHDT-Branch: NetHack-3.7 $:$NHDT-Revision: 1.188 $ */
+/* NetHack 3.7  makedefs.c  $NHDT-Date: 1639622361 2021/12/16 02:39:21 $  $NHDT-Branch: NetHack-3.7 $:$NHDT-Revision: 1.207 $ */
 /* Copyright (c) Stichting Mathematisch Centrum, Amsterdam, 1985. */
 /*-Copyright (c) Kenneth Lorber, Kensington, Maryland, 2015. */
 /* Copyright (c) M. Stephenson, 1990, 1991.                       */
@@ -8,9 +8,6 @@
 #define MAKEDEFS_C /* use to conditionally include file sections */
 
 #include "config.h"
-#ifdef MONITOR_HEAP
-#undef free /* makedefs doesn't use the alloc and free in src/alloc.c */
-#endif
 #include "permonst.h"
 #include "objclass.h"
 #include "sym.h"
@@ -174,6 +171,40 @@ static boolean use_enum = TRUE;
 extern unsigned _stklen = STKSIZ;
 #endif
 
+#ifdef MONITOR_HEAP
+/* for monitor heap, free(ptr) is a macro that expands to
+   nhfree(ptr,__FILE__,__LINE__) and nhfree() calls actual free();
+   makedefs wants to call actual free() so get rid of the macro */
+#undef free
+
+/* makedefs doesn't use alloc() from src/alloc.c but it links with
+   src/date.o which calls free() hence nhfree() if MONITOR_HEAP is
+   enabled; provide a substitute to satisfy the linker */
+void
+nhfree(genericptr_t ptr, const char *file UNUSED, int line UNUSED)
+{
+    free(ptr);
+}
+
+/* likewise for dupstr() which is also used in src/date.c */
+#undef dupstr
+
+char *
+nhdupstr(const char *string, const char *file UNUSED, int line UNUSED)
+{
+    return strcpy((char *) malloc(strlen(string) + 1), string);
+}
+#else /* !MONITOR_HEAP */
+
+/* without MONITOR_HEAP, we don't need to undef dupstr or provide
+   nhdupstr() but we do still need to provide a substitute dupstr() */
+char *
+dupstr(const char *string)
+{
+    return strcpy((char *) malloc(strlen(string) + 1), string);
+}
+#endif /* ?MONITOR_HEAP */
+
 /*
  * Some of the routines in this source file were moved into .../src/mdlib
  * to facilitate the use of a cross-compiler generation of some of the
@@ -2238,6 +2269,7 @@ tmpdup(const char* str)
     return buf;
 }
 
+/* the STRICT_REF_DEF checking is way out of date... */
 #ifdef STRICT_REF_DEF
 NEARDATA struct flag flags;
 #ifdef ATTRIB_H

diff --git a/win/share/tilemap.c b/win/share/tilemap.c
index 77aa3f147250ffcce6bf3a4f5f73de9370474b90..66a862545119388e583f0a07e1e893a0ca14af06 100644 (file)
--- a/win/share/tilemap.c
+++ b/win/share/tilemap.c
@@ -1,4 +1,4 @@
-/* NetHack 3.7 tilemap.c       $NHDT-Date: 1596498340 2020/08/03 23:45:40 $  $NHDT-Branch: NetHack-3.7 $:$NHDT-Revision: 1.47 $ */
+/* NetHack 3.7 tilemap.c       $NHDT-Date: 1639622363 2021/12/16 02:39:23 $  $NHDT-Branch: NetHack-3.7 $:$NHDT-Revision: 1.63 $ */
 /*      Copyright (c) 2016 by Michael Allison                     */
 /* NetHack may be freely redistributed.  See license for details. */
 
@@ -22,6 +22,13 @@
 #include <stdarg.h>
 #endif
 
+#ifdef MONITOR_HEAP
+/* with heap monitoring enabled, free(ptr) is a macro which expands to
+   nhfree(ptr,__FILE__,__LINE__); since tilemap doesn't link with
+   src/alloc.o it doesn't have access to nhfree(); use actual free */
+#undef free
+#endif
+
 #define Fprintf (void) fprintf
 #define Snprintf(str, size, ...) \
     nh_snprintf(__func__, __LINE__, str, size, __VA_ARGS__)
@@ -1423,8 +1430,13 @@ precheck(int offset, const char *glyphtype)
                 glyphtype);
 }
 
-void add_tileref(int n, int glyphref, enum tilesrc src, int entrynum,
-                 const char *nam, const char *prefix)
+void add_tileref(
+    int n,
+    int glyphref,
+    enum tilesrc src,
+    int entrynum,
+    const char *nam,
+    const char *prefix)
 {
     struct tiles_used temp = { 0 };
     static const char ellipsis[] UNUSED = "...";
@@ -1476,8 +1488,7 @@ free_tilerefs(void)
 
     for (i = 0; i < SIZE(tilelist); i++) {
         if (tilelist[i])
-            free(tilelist[i]);
-        tilelist[i] = (struct tiles_used *) 0;
+            free((genericptr_t) tilelist[i]), tilelist[i] = 0;
     }
 }