Re: [PATCH 00/22] add the FORMATPRINTF macro to declare the gcc function

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Jeff King <peff@xxxxxxxx> writes:

> On Thu, Feb 11, 2016 at 09:59:21AM -0800, Junio C Hamano wrote:
>
>> Elia Pinto <gitter.spiros@xxxxxxxxx> writes:
>> 
>> > Add the FORMATPRINTF macro for declaring the gcc function attribute 'format printf'
>> > for code style consistency with similar macro that git already use for other gcc
>> > attributes. And use it where necessary.
>> >
>> > Elia Pinto (22):
>> >   git-compat-util.h: add the FORMATPRINTF macro
>> 
>> Hmm.  Given that we already have
>> 
>> #ifndef __GNUC__
>> #ifndef __attribute__
>> #define __attribute__(x)
>> #endif
>> #endif
>> 
>> in git-compat-util.h, it is really between:
>> 
>>     __attribute__((format (printf, 1, 2)))
>>     void advise(const char *advice, ...);
>> 
>>     __attribute__((format (printf,2,3)))
>>     extern void strbuf_addf(struct strbuf *sb, const char *fmt, ...);
>> 
>> and
>> 
>>     FORMATPRINTF(1,2)
>>     void advise(const char *advice, ...);
>> 
>>     FORMATPRINTF(2,3)
>>     extern void strbuf_addf(struct strbuf *sb, const char *fmt, ...);
>> 
>> 
>> Perhaps I am biased for staring at our source code for too long, but
>> somehow the latter looks unnecessarily loud, spelled in all caps.
>> 
>> I dunno.  What does this really buy us?
>
> I had the same thought on reading this. I think the "similar macro" Elia
> mentions is probably NORETURN. But in that case, we cannot rely on
> __attribute__, because it is spelled so many different ways (e.g.,
> __declspec(noreturn)).
>
> This series would be helpful to us if there was a platform that
> understood the format attribute, but needed to spell it differently
> somehow. But short of that, I think it is a net negative.

Yes, exactly.

I've been carrying this in my tree for about a week, but I think
this does not buy us very much.  Let's drop it.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]