From 03c19762d67db5c2822f551dcf7bce11865b88c2 Mon Sep 17 00:00:00 2001 From: nhmall Date: Sat, 11 Jun 2022 00:18:27 -0400 Subject: [PATCH] new warnings showed up in old code MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A test build with gcc-12 cause two new warnings to appear. mkmaze.c: In function ‘makemaz’: mkmaze.c:983:44: warning: ‘sprintf’ may write a terminating nul past the end of the destination [-Wformat-overflow=] 983 | Sprintf(protofile, "%s%d-%d", g.dungeons[u.uz.dnum].proto, | ^ In file included from ../include/config.h:665, from ../include/hack.h:10, from mkmaze.c:6: ../include/global.h:262:24: note: ‘sprintf’ output between 4 and 31 bytes into a destination of size 20 262 | #define Sprintf (void) sprintf mkmaze.c:983:17: note: in expansion of macro ‘Sprintf’ 983 | Sprintf(protofile, "%s%d-%d", g.dungeons[u.uz.dnum].proto, | ^~~~~~~-+ As usual, that one can easily be rectified by replacing it with an Snprintf() call. There were several Sprintf calls in the vicinity, targeting the same destination buffer, so I figured that I might as well replace the several. ../win/Qt/qt_menu.cpp: In member function ‘virtual void nethack_qt_::NetHackQtTextWindow::UseRIP(int, time_t)’: ../win/Qt/qt_menu.cpp:1082:63: warning: ‘%ld’ directive output may be truncated writing between 1 and 20 bytes into a region of size 17 [-Wformat-truncation=] 1082 | (void) snprintf(rip_line[GOLD_LINE], STONE_LINE_LEN + 1, "%ld Au", cash); | ^~~ ../win/Qt/qt_menu.cpp:1082:62: note: directive argument in the range [-9223372036854775808, 999999999] 1082 | (void) snprintf(rip_line[GOLD_LINE], STONE_LINE_LEN + 1, "%ld Au", cash); | ^~~~~~~~ ../win/Qt/qt_menu.cpp:1082:20: note: ‘snprintf’ output between 5 and 24 bytes into a destination of size 17 1082 | (void) snprintf(rip_line[GOLD_LINE], STONE_LINE_LEN + 1, "%ld Au", cash); | ~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ That one was a little different. It wasn't complaining about the destination buffer size in the way -Wformat-overflow was on the previous warning from gcc, and it was already using snprintf(). It looks like what the C++ compiler was warning about there, was that snprintf() informs you after-the-call that the destination buffer was too small for the result string to be fully written. It informs the developer of that by returning the number of characters that would have been written if the buffer had been big enough. Presumably, the C++ compiler picked up on the fact that the return value was being cast to void, thus throwing away that truncation information from the return value. Worked around it by putting the return value into a variable, and flagging the variable with nhUse(variable) so as not to exchange the -Wformat-truncation warning with a variable-set-but-not-used warning. --- src/mkmaze.c | 18 +++++++++++------- win/Qt/qt_menu.cpp | 6 +++--- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/src/mkmaze.c b/src/mkmaze.c index ea1ebdae4..ae0961413 100644 --- a/src/mkmaze.c +++ b/src/mkmaze.c @@ -974,20 +974,24 @@ makemaz(const char *s) if (*s) { if (sp && sp->rndlevs) - Sprintf(protofile, "%s-%d", s, rnd((int) sp->rndlevs)); + Snprintf(protofile, sizeof protofile, + "%s-%d", s, rnd((int) sp->rndlevs)); else Strcpy(protofile, s); } else if (*(g.dungeons[u.uz.dnum].proto)) { if (dunlevs_in_dungeon(&u.uz) > 1) { if (sp && sp->rndlevs) - Sprintf(protofile, "%s%d-%d", g.dungeons[u.uz.dnum].proto, - dunlev(&u.uz), rnd((int) sp->rndlevs)); + Snprintf(protofile, sizeof protofile, + "%s%d-%d", g.dungeons[u.uz.dnum].proto, + dunlev(&u.uz), rnd((int) sp->rndlevs)); else - Sprintf(protofile, "%s%d", g.dungeons[u.uz.dnum].proto, - dunlev(&u.uz)); + Snprintf(protofile, sizeof protofile, + "%s%d", g.dungeons[u.uz.dnum].proto, + dunlev(&u.uz)); } else if (sp && sp->rndlevs) { - Sprintf(protofile, "%s-%d", g.dungeons[u.uz.dnum].proto, - rnd((int) sp->rndlevs)); + Snprintf(protofile, sizeof protofile, + "%s-%d", g.dungeons[u.uz.dnum].proto, + rnd((int) sp->rndlevs)); } else Strcpy(protofile, g.dungeons[u.uz.dnum].proto); diff --git a/win/Qt/qt_menu.cpp b/win/Qt/qt_menu.cpp index 7a4eefee2..bc6810b0f 100644 --- a/win/Qt/qt_menu.cpp +++ b/win/Qt/qt_menu.cpp @@ -1058,7 +1058,7 @@ void NetHackQtTextWindow::UseRIP(int how, time_t when) char buf[BUFSZ]; char *dpx; - int line; + int line, snpres; /* Put name on stone */ (void) snprintf(rip_line[NAME_LINE], STONE_LINE_LEN + 1, @@ -1079,8 +1079,8 @@ void NetHackQtTextWindow::UseRIP(int how, time_t when) it's arbitrary but still way, way more than could ever be needed */ if (cash > 999999999L) cash = 999999999L; - (void) snprintf(rip_line[GOLD_LINE], STONE_LINE_LEN + 1, "%ld Au", cash); - + snpres = snprintf(rip_line[GOLD_LINE], STONE_LINE_LEN + 1, "%ld Au", cash); + nhUse(snpres); /* Put together death description */ formatkiller(buf, sizeof buf, how, FALSE); //str_copy(buf, killer, SIZE(buf));