From 0086e0196733d32ddd2c14638cd7d0e221a5f6de Mon Sep 17 00:00:00 2001 From: nhmall Date: Tue, 6 Sep 2022 10:00:07 -0400 Subject: [PATCH] return legal indexes for some display.h macros Some static analyzers flagged the last-resort values as out of bounds (which they were). There's a small number of other complaint-suppression items in here too, but nothing drastic. --- include/display.h | 9 ++- include/sym.h | 2 +- src/apply.c | 3 +- src/display.c | 152 ++++++++++++++++++++++---------------------- src/drawing.c | 9 ++- win/share/tilemap.c | 2 +- 6 files changed, 93 insertions(+), 84 deletions(-) diff --git a/include/display.h b/include/display.h index a427d1e1b..f8f8585dd 100644 --- a/include/display.h +++ b/include/display.h @@ -710,6 +710,7 @@ enum glyph_offsets { ((glyph) >= GLYPH_CMAP_STONE_OFF \ && (glyph) < (GLYPH_CMAP_C_OFF + ((S_goodpos - S_digbeam) + 1))) +/* final MAXPCHARS is legal array index because of trailing fencepost entry */ #define glyph_to_cmap(glyph) \ (((glyph) == GLYPH_CMAP_STONE_OFF) \ ? S_stone \ @@ -733,7 +734,7 @@ enum glyph_offsets { ? (((glyph) - GLYPH_CMAP_C_OFF) + S_digbeam) \ : glyph_is_cmap_zap(glyph) \ ? ((((glyph) - GLYPH_ZAP_OFF) % 4) + S_vbeam) \ - : NO_GLYPH) + : MAXPCHARS) #define glyph_to_swallow(glyph) \ (glyph_is_swallow(glyph) ? (((glyph) - GLYPH_SWALLOW_OFF) & 0x7) : 0) @@ -779,6 +780,7 @@ enum glyph_offsets { || glyph_is_ridden_monster(glyph) || glyph_is_detected_monster(glyph)) #define glyph_is_invisible(glyph) ((glyph) == GLYPH_INVISIBLE) +/* final NUMMONS is legal array index because of trailing fencepost entry */ #define glyph_to_mon(glyph) \ (glyph_is_normal_female_monster(glyph) \ ? ((glyph) - GLYPH_MON_FEM_OFF) \ @@ -796,7 +798,7 @@ enum glyph_offsets { ? ((glyph) - GLYPH_RIDDEN_FEM_OFF) \ : glyph_is_ridden_male_monster(glyph) \ ? ((glyph) - GLYPH_RIDDEN_MALE_OFF) \ - : NO_GLYPH) + : NUMMONS) #define obj_is_piletop(obj) \ ((obj)->where == OBJ_FLOOR \ @@ -876,6 +878,7 @@ enum glyph_offsets { ? (GLYPH_BODY_OFF))) \ : ((int) (obj)->otyp + GLYPH_OBJ_OFF)))) +/* final NUM_OBJECTS is legal array idx because of trailing fencepost entry */ #define glyph_to_obj(glyph) \ (glyph_is_body(glyph) ? CORPSE \ : glyph_is_statue(glyph) ? STATUE \ @@ -883,7 +886,7 @@ enum glyph_offsets { ? ((glyph) - (glyph_is_normal_piletop_obj(glyph) \ ? GLYPH_OBJ_PILETOP_OFF \ : GLYPH_OBJ_OFF)) \ - : NO_GLYPH) + : NUM_OBJECTS) #define glyph_to_body_corpsenm(glyph) \ (glyph_is_body_piletop(glyph) \ diff --git a/include/sym.h b/include/sym.h index 5192404ce..51f398a73 100644 --- a/include/sym.h +++ b/include/sym.h @@ -160,7 +160,7 @@ struct symset_customization { }; #endif /* ENHANCED_SYMBOLS */ -extern const struct symdef defsyms[MAXPCHARS]; /* defaults */ +extern const struct symdef defsyms[MAXPCHARS + 1]; /* defaults */ #define WARNCOUNT 6 /* number of different warning levels */ extern const struct symdef def_warnsyms[WARNCOUNT]; #define SYMHANDLING(ht) (g.symset[g.currentgraphics].handling == (ht)) diff --git a/src/apply.c b/src/apply.c index 09da24177..10dd26ac6 100644 --- a/src/apply.c +++ b/src/apply.c @@ -3161,13 +3161,12 @@ static boolean find_poleable_mon(coord *pos, int min_range, int max_range) { struct monst *mtmp; - coord mpos; + coord mpos = { 0, 0 }; /* no candidate location yet */ boolean impaired; coordxy x, y, lo_x, hi_x, lo_y, hi_y, rt; int glyph; impaired = (Confusion || Stunned || Hallucination); - mpos.x = mpos.y = 0; /* no candidate location yet */ rt = isqrt(max_range); lo_x = max(u.ux - rt, 1), hi_x = min(u.ux + rt, COLNO - 1); lo_y = max(u.uy - rt, 0), hi_y = min(u.uy + rt, ROWNO - 1); diff --git a/src/display.c b/src/display.c index 6660779a1..fc6a6474f 100644 --- a/src/display.c +++ b/src/display.c @@ -1075,90 +1075,92 @@ tmp_at(coordxy x, coordxy y) break; } - if (!tglyph) + if (!tglyph) { panic("tmp_at: tglyph not initialized"); + } else { + switch (x) { + case DISP_CHANGE: + tglyph->glyph = y; + break; - switch (x) { - case DISP_CHANGE: - tglyph->glyph = y; - break; + case DISP_END: + if (tglyph->style == DISP_BEAM || tglyph->style == DISP_ALL) { + register int i; - case DISP_END: - if (tglyph->style == DISP_BEAM || tglyph->style == DISP_ALL) { - register int i; - - /* Erase (reset) from source to end */ - for (i = 0; i < tglyph->sidx; i++) - newsym(tglyph->saved[i].x, tglyph->saved[i].y); - } else if (tglyph->style == DISP_TETHER) { - int i; - - if (y == BACKTRACK && tglyph->sidx > 1) { - /* backtrack */ - for (i = tglyph->sidx - 1; i > 0; i--) { + /* Erase (reset) from source to end */ + for (i = 0; i < tglyph->sidx; i++) newsym(tglyph->saved[i].x, tglyph->saved[i].y); - show_glyph(tglyph->saved[i - 1].x, - tglyph->saved[i - 1].y, tglyph->glyph); - flush_screen(0); /* make sure it shows up */ - delay_output(); + } else if (tglyph->style == DISP_TETHER) { + int i; + + if (y == BACKTRACK && tglyph->sidx > 1) { + /* backtrack */ + for (i = tglyph->sidx - 1; i > 0; i--) { + newsym(tglyph->saved[i].x, tglyph->saved[i].y); + show_glyph(tglyph->saved[i - 1].x, + tglyph->saved[i - 1].y, tglyph->glyph); + flush_screen(0); /* make sure it shows up */ + delay_output(); + } + tglyph->sidx = 1; } + for (i = 0; i < tglyph->sidx; i++) + newsym(tglyph->saved[i].x, tglyph->saved[i].y); + } else { /* DISP_FLASH or DISP_ALWAYS */ + if (tglyph->sidx) /* been called at least once */ + newsym(tglyph->saved[0].x, tglyph->saved[0].y); + } + /* tglyph->sidx = 0; -- about to be freed, so not necessary */ + tmp = tglyph->prev; + if (tglyph != &tgfirst) + free((genericptr_t) tglyph); + tglyph = tmp; + break; + + default: /* do it */ + if (!isok(x, y)) + break; + if (tglyph->style == DISP_BEAM || tglyph->style == DISP_ALL) { + if (tglyph->style != DISP_ALL && !cansee(x, y)) + break; + if (tglyph->sidx >= TMP_AT_MAX_GLYPHS) + break; /* too many locations */ + /* save pos for later erasing */ + tglyph->saved[tglyph->sidx].x = x; + tglyph->saved[tglyph->sidx].y = y; + tglyph->sidx += 1; + } else if (tglyph->style == DISP_TETHER) { + if (tglyph->sidx >= TMP_AT_MAX_GLYPHS) + break; /* too many locations */ + if (tglyph->sidx) { + int px, py; + + px = tglyph->saved[tglyph->sidx - 1].x; + py = tglyph->saved[tglyph->sidx - 1].y; + show_glyph(px, py, tether_glyph(px, py)); + } + /* save pos for later use or erasure */ + tglyph->saved[tglyph->sidx].x = x; + tglyph->saved[tglyph->sidx].y = y; + tglyph->sidx += 1; + } else { /* DISP_FLASH/ALWAYS */ + if (tglyph + ->sidx) { /* not first call, so reset previous pos */ + newsym(tglyph->saved[0].x, tglyph->saved[0].y); + tglyph->sidx = 0; /* display is presently up to date */ + } + if (!cansee(x, y) && tglyph->style != DISP_ALWAYS) + break; + tglyph->saved[0].x = x; + tglyph->saved[0].y = y; tglyph->sidx = 1; } - for (i = 0; i < tglyph->sidx; i++) - newsym(tglyph->saved[i].x, tglyph->saved[i].y); - } else { /* DISP_FLASH or DISP_ALWAYS */ - if (tglyph->sidx) /* been called at least once */ - newsym(tglyph->saved[0].x, tglyph->saved[0].y); - } - /* tglyph->sidx = 0; -- about to be freed, so not necessary */ - tmp = tglyph->prev; - if (tglyph != &tgfirst) - free((genericptr_t) tglyph); - tglyph = tmp; - break; - default: /* do it */ - if (!isok(x, y)) + show_glyph(x, y, tglyph->glyph); /* show it */ + flush_screen(0); /* make sure it shows up */ break; - if (tglyph->style == DISP_BEAM || tglyph->style == DISP_ALL) { - if (tglyph->style != DISP_ALL && !cansee(x, y)) - break; - if (tglyph->sidx >= TMP_AT_MAX_GLYPHS) - break; /* too many locations */ - /* save pos for later erasing */ - tglyph->saved[tglyph->sidx].x = x; - tglyph->saved[tglyph->sidx].y = y; - tglyph->sidx += 1; - } else if (tglyph->style == DISP_TETHER) { - if (tglyph->sidx >= TMP_AT_MAX_GLYPHS) - break; /* too many locations */ - if (tglyph->sidx) { - int px, py; - - px = tglyph->saved[tglyph->sidx-1].x; - py = tglyph->saved[tglyph->sidx-1].y; - show_glyph(px, py, tether_glyph(px, py)); - } - /* save pos for later use or erasure */ - tglyph->saved[tglyph->sidx].x = x; - tglyph->saved[tglyph->sidx].y = y; - tglyph->sidx += 1; - } else { /* DISP_FLASH/ALWAYS */ - if (tglyph->sidx) { /* not first call, so reset previous pos */ - newsym(tglyph->saved[0].x, tglyph->saved[0].y); - tglyph->sidx = 0; /* display is presently up to date */ - } - if (!cansee(x, y) && tglyph->style != DISP_ALWAYS) - break; - tglyph->saved[0].x = x; - tglyph->saved[0].y = y; - tglyph->sidx = 1; - } - - show_glyph(x, y, tglyph->glyph); /* show it */ - flush_screen(0); /* make sure it shows up */ - break; - } /* end case */ + } /* end switch */ + } } /* diff --git a/src/drawing.c b/src/drawing.c index bba568f94..4db886acb 100644 --- a/src/drawing.c +++ b/src/drawing.c @@ -66,10 +66,15 @@ const struct symdef def_warnsyms[WARNCOUNT] = { * parsing other.txt because some of the useful tile names don't exist * within NetHack itself. */ -const struct symdef defsyms[MAXPCHARS] = { +const struct symdef defsyms[MAXPCHARS + 1] = { #define PCHAR_DRAWING #include "defsym.h" #undef PCHAR_DRAWING + { 0, NULL +#ifdef TEXTCOLOR + , NO_COLOR +#endif + } }; /* default rogue level symbols */ @@ -130,7 +135,7 @@ def_char_is_furniture(char ch) int i; boolean furniture = FALSE; - for (i = 0; i < SIZE(defsyms); ++i) { + for (i = 0; i < MAXPCHARS; ++i) { if (!furniture) { if (!strncmp(defsyms[i].explanation, first_furniture, 5)) furniture = TRUE; diff --git a/win/share/tilemap.c b/win/share/tilemap.c index b9464191d..ad322f55e 100644 --- a/win/share/tilemap.c +++ b/win/share/tilemap.c @@ -62,7 +62,7 @@ struct { int idx; const char *tilelabel; const char *expectedlabel; -} altlabels[MAXPCHARS] = { +} altlabels[MAXPCHARS + 1] = { #define PCHAR_TILES #include "defsym.h" #undef PCHAR_TILES