Merge branch 'glyphs-fixes-take2' of https://github.com/ingpaschke/NetHack into pr1559

src/glyphs.c

@@ -28,7 +28,7 @@ struct find_struct {
 static const struct find_struct zero_find = { 0 };
 struct glyphname_hash_index_entry_t {
     uint32 hash;
-    int glyphnum;       /* NO_GLYPH (==MAX_GLYPH) marks an empty bucket */
+    int glyphnum;
 };
 static struct glyphname_hash_index_entry_t *glyphname_hash_indices_ptr;
 static size_t glyphname_hash_indices_count;
@@ -39,6 +39,7 @@ staticfn int find_glyph_in_hashtable(const char *id);
 staticfn int cmp_glyphname_entry(const void *a, const void *b);
 staticfn uint32 glyph_hash(const char *id);
 staticfn int compose_glyph_name(int glyph, char *buf, size_t bufsz);
+staticfn int get_cmap_offset(void);
 staticfn void to_custom_symset_entry_callback(int glyph,
                                             struct find_struct *findwhat);
 staticfn int parse_id(const char *id, struct find_struct *findwhat);
@@ -194,32 +195,46 @@ fix_glyphname(char *str)
     return str;
 }
 
+/* First SYM_PCHAR index in loadsyms[], cached (0 = not computed yet;
+   SYM_PCHAR is never at index 0). */
+static int cached_cmap_offset = 0;
+
+staticfn int
+get_cmap_offset(void)
+{
+    if (!cached_cmap_offset) {
+        int i;
+
+        for (i = 0; loadsyms[i].range; i++) {
+            if (loadsyms[i].range == SYM_PCHAR) {
+                cached_cmap_offset = i;
+                break;
+            }
+        }
+    }
+    return cached_cmap_offset;
+}
+
 /* Build the canonical "G_xxx" identifier for the given glyph into buf.
  * Returns 1 if a name was produced, 0 if this glyph has no canonical name
  * (a few unused object indices); buf[0] is set to '\0' in that case.
- * Used by parse_id's bulk-iteration paths, by find_glyph_in_hashtable to
+ * Callers must pass a BUFSZ-sized buffer.
- * verify hash matches, by populate_glyphname_hash_indices to fill the table,
+ * Used by find_glyph_in_hashtable to verify hash matches, by the
- * and by wizcustom_glyphnames. */
+ * --dumpglyphnames path, by populate_glyphname_hash_indices to fill the
+ * table, and by wizcustom_glyphnames. */
 staticfn int
 compose_glyph_name(int glyph, char *buf, size_t bufsz)
 {
-    int i, j, mnum, cmap_offset = 0;
+    int i, j, mnum, cmap_offset;
     boolean skip_base = FALSE;
     const char *buf2, *buf3, *buf4;
     char tmpbuf[4][QBUFSZ];
 
-    if (bufsz < 2)
+    if (bufsz < BUFSZ)
         return 0;
     buf[0] = '\0';
     tmpbuf[0][0] = tmpbuf[1][0] = tmpbuf[2][0] = tmpbuf[3][0] = '\0';
-
+    cmap_offset = get_cmap_offset();
-    /* compute cmap_offset (the start of SYM_PCHAR entries in loadsyms[]) */
-    i = 0;
-    while (loadsyms[i].range) {
-        if (!cmap_offset && loadsyms[i].range == SYM_PCHAR)
-            cmap_offset = i;
-        i++;
-    }
 
     if (glyph_is_monster(glyph)) {
         buf2 = "";
@@ -241,15 +256,11 @@ compose_glyph_name(int glyph, char *buf, size_t bufsz)
         } else if (glyph_is_female_pet(glyph)) {
             buf2 = "pet_female_";
         }
-        Strcpy(buf, "G_");
+        Snprintf(buf, bufsz, "G_%s%s", buf2, buf3);
-        Strcat(buf, buf2);
-        Strcat(buf, buf3);
     } else if (glyph_is_body(glyph)) {
         buf2 = glyph_is_body_piletop(glyph) ? "piletop_body_" : "body_";
         buf3 = monsdump[glyph_to_body_corpsenm(glyph)].nm;
-        Strcpy(buf, "G_");
+        Snprintf(buf, bufsz, "G_%s%s", buf2, buf3);
-        Strcat(buf, buf2);
-        Strcat(buf, buf3);
     } else if (glyph_is_statue(glyph)) {
         buf2 = glyph_is_fem_statue_piletop(glyph)
                ? "piletop_statue_of_female_"
@@ -261,9 +272,7 @@ compose_glyph_name(int glyph, char *buf, size_t bufsz)
                      ? "statue_of_male_"
                      : "";
         buf3 = monsdump[glyph_to_statue_corpsenm(glyph)].nm;
-        Strcpy(buf, "G_");
+        Snprintf(buf, bufsz, "G_%s%s", buf2, buf3);
-        Strcat(buf, buf2);
-        Strcat(buf, buf3);
     } else if (glyph_is_object(glyph)) {
         i = glyph_to_obj(glyph);
         if (((i > SCR_STINKING_CLOUD) && (i < SCR_MAIL))
@@ -290,12 +299,10 @@ compose_glyph_name(int glyph, char *buf, size_t bufsz)
                    : obj_descr[i].oc_name
                      ? obj_descr[i].oc_name
                      : obj_descr[i].oc_descr;
-        Strcpy(buf, "G_");
+        Snprintf(buf, bufsz, "G_%s%s%s",
-        if (glyph_is_normal_piletop_obj(glyph)
+                 (glyph_is_normal_piletop_obj(glyph)
-            || glyph_is_piletop_generic_obj(glyph))
+                  || glyph_is_piletop_generic_obj(glyph)) ? "piletop_" : "",
-            Strcat(buf, "piletop_");
+                 buf2, buf3);
-        Strcat(buf, buf2);
-        Strcat(buf, buf3);
     } else if (glyph_is_cmap(glyph) || glyph_is_cmap_zap(glyph)
                || glyph_is_swallow(glyph) || glyph_is_explosion(glyph)) {
         int cmap = -1;
@@ -368,10 +375,8 @@ compose_glyph_name(int glyph, char *buf, size_t bufsz)
             j = glyph - GLYPH_SWALLOW_OFF;
             cmap = glyph_to_swallow(glyph);
             mnum = j / ((S_sw_br - S_sw_tl) + 1);
-            Strcpy(tmpbuf[3], "swallow ");
+            Snprintf(tmpbuf[3], sizeof tmpbuf[3], "swallow %s %s",
-            Strcat(tmpbuf[3], monsdump[mnum].nm);
+                     monsdump[mnum].nm, swallow_texts[cmap]);
-            Strcat(tmpbuf[3], " ");
-            Strcat(tmpbuf[3], swallow_texts[cmap]);
             buf3 = tmpbuf[3];
             skip_base = TRUE;
         } else if (glyph_is_explosion(glyph)) {
@@ -401,16 +406,13 @@ compose_glyph_name(int glyph, char *buf, size_t bufsz)
             if (cmap >= 0 && cmap < MAXPCHARS)
                 buf3 = loadsyms[cmap + cmap_offset].name + 2;
         }
-        Strcpy(buf, "G_");
+        Snprintf(buf, bufsz, "G_%s%s%s", buf2, buf3, buf4);
-        Strcat(buf, buf2);
-        Strcat(buf, buf3);
-        Strcat(buf, buf4);
     } else if (glyph_is_invisible(glyph)) {
-        Strcpy(buf, "G_invisible");
+        Snprintf(buf, bufsz, "G_invisible");
     } else if (glyph_is_nothing(glyph)) {
-        Strcpy(buf, "G_nothing");
+        Snprintf(buf, bufsz, "G_nothing");
     } else if (glyph_is_unexplored(glyph)) {
-        Strcpy(buf, "G_unexplored");
+        Snprintf(buf, bufsz, "G_unexplored");
     } else if (glyph_is_warning(glyph)) {
         j = glyph - GLYPH_WARNING_OFF;
         Snprintf(buf, bufsz, "G_%s%d", "warning", j);
@@ -418,11 +420,6 @@ compose_glyph_name(int glyph, char *buf, size_t bufsz)
 
     if (buf[0] == '\0')
         return 0;
-    if (memchr(buf, '\0', bufsz) == NULL) {
-        /* defensive: caller passed an undersized buffer */
-        buf[bufsz - 1] = '\0';
-        return 0;
-    }
     fix_glyphname(buf + 2);
     nhUse(mnum);
     return 1;
@@ -1036,10 +1033,8 @@ parse_id(
         }
     }
     if (is_G && id) {
-        /* Populate the hash table lazily, on first G_xxx lookup. */
-        if (!glyphname_hash_indices_ptr)
-            populate_glyphname_hash_indices();
         if (glyphname_hash_indices_ptr) {
+            /* Fast path: bsearch the populated index. */
             int val = find_glyph_in_hashtable(id);
 
             if (val >= 0) {
@@ -1048,6 +1043,17 @@ parse_id(
                 findwhat->loadsyms_offset = 0;
                 return 1;
             }
+        } else {
+            /* Unpopulated: linear scan, no alloc. */
+            for (glyph = 0; glyph < MAX_GLYPH; ++glyph) {
+                if (compose_glyph_name(glyph, buf, sizeof buf)
+                    && !strcmpi(id, buf)) {
+                    findwhat->findtype = find_glyph;
+                    findwhat->val = glyph;
+                    findwhat->loadsyms_offset = 0;
+                    return 1;
+                }
+            }
         }
         return 0;
     }
@@ -1079,7 +1085,7 @@ parse_id(
             }
         }
         /* permonst entries */
-        for (i = 0; i <= pm_count; ++i) {
+        for (i = 0; i < pm_count; ++i) {
             if (!strcmpi(loadsyms[i + pm_offset].name + 2, id + 2)) {
                 findwhat->findtype = find_pm;
                 findwhat->val = i + 1; /* starts at 1 */