Re: [PATCH 00/22] add the FORMATPRINTF macro to declare the gcc function
- From
Jeff King <peff@peff.net>
- Date
- Feb 11, 2016, 22:33 UTC
- Message-ID
- <20160211223323.GB641@sigill.intra.peff.net>
- In-Reply-To
- <xmqq37szxh52.fsf@gitster.mtv.corp.google.com>
On Thu, Feb 11, 2016 at 09:59:21AM -0800, Junio C Hamano wrote:
Show 38 quoted lines
> 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