nethack-gwaa-kiwi/nethack
1
0
Fork 0
Code Pull Requests Activity

glyphs: code-review fixes and parse_id off-by-one

Browse Source 
- cmap_offset was calculated on every compose_glyph_name() call;
    calculate it once with a static function.
  - Drop parse_id's G_ auto-populate: an unbracketed lookup allocated
    the index with no matching free.  Linear-scan instead when absent.
  - compose_glyph_name: require bufsz >= BUFSZ, build names with
    bounded Snprintf instead of Strcpy/Strcat, drop the dead memchr.
  - parse_id's permonst scan used i <= pm_count, reading one past the
    SYM_MON block (S_nothing) and matching it as a monster; use <.
  - Drop a stale NO_GLYPH empty-bucket comment from the open-addressed
    table.
This commit is contained in:
Ingo Paschke
2026-05-28 21:08:54 +02:00
parent a9401fd78a
commit 5f817fe9ad
1 changed files with 55 additions and 49 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/glyphs.c
+55 -49
View File
@@ -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
* verify hash matches, by populate_glyphname_hash_indices to fill the table,
* and by wizcustom_glyphnames. */
* Callers must pass a BUFSZ-sized buffer.
* Used by find_glyph_in_hashtable to verify hash matches, by the
* --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';
/* 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++;
}
cmap_offset = get_cmap_offset();
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_");
Strcat(buf, buf2);
Strcat(buf, buf3);
Snprintf(buf, bufsz, "G_%s%s", buf2, 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_");
Strcat(buf, buf2);
Strcat(buf, buf3);
Snprintf(buf, bufsz, "G_%s%s", buf2, 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_");
Strcat(buf, buf2);
Strcat(buf, buf3);
Snprintf(buf, bufsz, "G_%s%s", buf2, 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_");
if (glyph_is_normal_piletop_obj(glyph)
|| glyph_is_piletop_generic_obj(glyph))
Strcat(buf, "piletop_");
Strcat(buf, buf2);
Strcat(buf, buf3);
Snprintf(buf, bufsz, "G_%s%s%s",
(glyph_is_normal_piletop_obj(glyph)
|| glyph_is_piletop_generic_obj(glyph)) ? "piletop_" : "",
buf2, 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 ");
Strcat(tmpbuf[3], monsdump[mnum].nm);
Strcat(tmpbuf[3], " ");
Strcat(tmpbuf[3], swallow_texts[cmap]);
Snprintf(tmpbuf[3], sizeof tmpbuf[3], "swallow %s %s",
monsdump[mnum].nm, 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_");
Strcat(buf, buf2);
Strcat(buf, buf3);
Strcat(buf, buf4);
Snprintf(buf, bufsz, "G_%s%s%s", buf2, buf3, 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 */