From 44328d522bc56996cb93bdb852afa042d363afc5 Mon Sep 17 00:00:00 2001 From: Eldred Habert Date: Sat, 19 Sep 2026 03:15:07 +0200 Subject: [PATCH] Avoid using `snprintf` where GCC can check sizes (#2121) Interestingly, Clang's `-Wformat-overflow` seems to only warn about buffers so small they will *always* underflow, whereas GCC tries to infer whether they can overflow *at all*. (I'm guessing they lean towards false-negatives and false-positives resp.) Anyway, calling `sprintf` here remains safe, since GCC (notably, via CI) checks our work, and it simplifies the code ever so slightly while also providing an (admittedly negligible) performance improvement. Note that there may be more locations where we could use `sprintf`, but a cursory glance at our other uses of `snprintf` didn't seem fruitful. (One way to test is to set the target buffer size to 1, and see if a warning pops up for such a trivially-wrong size. If not, you can be certain the compiler won't be able to help you with a more realistic size.) --- include/platform.hpp | 22 ++++++++++++++++++++++ src/link/assign.cpp | 15 ++++++++------- 2 files changed, 30 insertions(+), 7 deletions(-) diff --git a/include/platform.hpp b/include/platform.hpp index fd5156f6..89f19aaf 100644 --- a/include/platform.hpp +++ b/include/platform.hpp @@ -66,4 +66,26 @@ #define _POSIX_C_SOURCE 200809L #endif +// Apple has deprecated `sprintf` since Xcode 14 (for macOS 13), but we use it solely in +// contexts where both the size of the buffer *and* max size of the printed string are +// known statically, which GCC thus checks for. +#ifdef __APPLE__ + #define sprintf_to_array(array, ...) \ + do { \ + static_assert( \ + std::is_array_v, "Only use this macro to print to an array!" \ + ); \ + snprintf(array, sizeof(array), __VA_ARGS__); \ + } while (0) + +#else + #define sprintf_to_array(array, ...) \ + do { \ + static_assert( \ + std::is_array_v, "Only use this macro to print to an array!" \ + ); \ + sprintf(array, __VA_ARGS__); \ + } while (0) +#endif + #endif // RGBDS_PLATFORM_HPP diff --git a/src/link/assign.cpp b/src/link/assign.cpp index 62a9acc9..63c6d758 100644 --- a/src/link/assign.cpp +++ b/src/link/assign.cpp @@ -16,6 +16,7 @@ #include "helpers.hpp" #include "itertools.hpp" #include "linkdefs.hpp" +#include "platform.hpp" #include "verbosity.hpp" #include "link/main.hpp" @@ -286,14 +287,14 @@ static std::string describeConstraintsOf(Section const §ion) { std::string description = "\"" + section.name + "\" (" + section.typeInfo().name + " section) "; if (section.isBankFixed && section.typeInfo().isBanked()) { char bank[9]; - snprintf(bank, sizeof(bank), "%02" PRIx32, section.bank); + sprintf_to_array(bank, "%02" PRIx32, section.bank); if (section.isAddressFixed) { char addr[5]; - snprintf(addr, sizeof(addr), "%04" PRIx16, section.org); + sprintf_to_array(addr, "%04" PRIx16, section.org); description = description + "at $" + bank + ":" + addr; } else if (section.isAlignFixed) { char mask[5]; - snprintf(mask, sizeof(mask), "%" PRIx16, static_cast(~section.alignMask)); + sprintf_to_array(mask, "%" PRIx16, static_cast(~section.alignMask)); description = description + "in bank $" + bank + " with align mask $" + mask; } else { description = description + "in bank $" + bank; @@ -301,12 +302,12 @@ static std::string describeConstraintsOf(Section const §ion) { } else { if (section.isAddressFixed) { char addr[5]; - snprintf(addr, sizeof(addr), "%04" PRIx16, section.org); + sprintf_to_array(addr, "%04" PRIx16, section.org); description = description + "at address $" + addr; } else if (section.isAlignFixed) { char mask[5], offset[5]; - snprintf(mask, sizeof(mask), "%" PRIx16, static_cast(~section.alignMask)); - snprintf(offset, sizeof(offset), "%" PRIx16, section.alignOfs); + sprintf_to_array(mask, "%" PRIx16, static_cast(~section.alignMask)); + sprintf_to_array(offset, "%" PRIx16, section.alignOfs); description = description + "with align mask $" + mask + " and offset $" + offset; } else { description = description + "anywhere"; @@ -315,7 +316,7 @@ static std::string describeConstraintsOf(Section const §ion) { if (auto info = scrambling.getInfoFor(section.type); info.has_value() && info->maxOfs != 0) { // Only mention scrambling if it is enabled. char size[6]; - snprintf(size, sizeof(size), "%" PRIu16, info->maxOfs); + sprintf_to_array(size, "%" PRIu16, info->maxOfs); description = description + " within the " + size + " scrambled banks"; } }