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

[Jeff King <peff@peff.net>]

On Thu, Feb 11, 2016 at 09:59:21AM -0800, Junio C Hamano wrote:

> Elia Pinto <gitter.spiros@gmail.com> 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.

-Peff