RE: [PATCH 6/6] git-compat-util.h: add __attribute__((printf)) to git_*printf*
On July 12, 2021 5:45 PM, Jeff King wrote:
Show 24 quoted lines
>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.