From: nhmall Date: Sat, 11 Jun 2022 04:18:27 +0000 (-0400) Subject: new warnings showed up in old code X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=03c19762d67db5c2822f551dcf7bce11865b88c2;p=nethack new warnings showed up in old code 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. --- 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));