Re: [PATCH 00/22] add the FORMATPRINTF macro to declare the gcc function
From: Junio C Hamano <hidden>
Date: 2016-06-15 23:08:18
Jeff King [off-list ref] writes:
On Thu, Feb 11, 2016 at 09:59:21AM -0800, Junio C Hamano wrote:quoted
Elia Pinto [off-list ref] writes:quoted
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 macroHmm. 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.