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