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 ea1ebdae445746870721ca2d4098125a46ea3531..ae0961413bd11a6397e6e0ce73952588afcd71e9 100644 (file)
--- 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 7a4eefee2897986714c13ab27948310dec204cd9..bc6810b0f0659f2013103b589e80f94c554dca88 100644 (file)
--- 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));