On July 12, 2021 5:45 PM, Jeff King wrote: >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). Works for me, in that case.
