Re: [PATCH 6/6] git-compat-util.h: add __attribute__((printf)) to git_*printf*
- From
Jeff King <peff@peff.net>
- Date
- Jul 12, 2021, 21:45 UTC
- Message-ID
- <YOy33I+T0LsLHYoS@coredump.intra.peff.net>
- In-Reply-To
- <012b01d7775c$2ddcca60$89965f20$@nexbridge.com>
On Mon, Jul 12, 2021 at 04:26:16PM -0400, Randall S. Becker wrote:
Show 19 quoted lines
> >> 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