Thread (1 message) 1 message, 1 author, 2016-06-15

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 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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help