On Mon, Jul 12, 2021 at 04:26:16PM -0400, Randall S. Becker wrote: > >> In practice this is redundant to the compiler's default printf format > >> checking, since we mostly (entirely?) develop and test on platforms > >> where SNPRINTF_RETURNS_BOGUS is not set. I'm doing this mainly for > >> consistency with other code, now we don't need to think about why this > >> particular case is different. > >> > >> See c4582f93a26 (Add compat/snprintf.c for systems that return bogus, > >> 2008-03-05) for the commit that added these functions. > > > >I'm slightly lukewarm on general on adding this to a compat function. > >Those are meant to be a lowest-common-denominator fallback, and we usually avoid fancy features or our usual styles there in favor of > >simplicity. > > > >I guess this probably isn't _hurting_ anything, but it makes me wonder how many systems have a broken snprintf _and_ support the > >attribute. > > NonStop does not support __attribute__ on any compiler I know of. This > appears to be a gcc extension, so compat.c would create a gcc > dependency, which is also not on the platform. snprintf is in place. We already turn __attribute__ into a noop on unsupported platforms early in git-compat-util.h (around line 443). So this would be OK, since the snprintf macro hackery is later in the file (around line 786). -Peff
