git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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
Previous: Junio C HamanoNext: Junio C Hamano
Message 27 of 28 in “add the FORMATPRINTF macro to declare the gcc function”
  1. 00/22 add the FORMATPRINTF macro to declare the gcc functionElia Pinto, Feb 11, 2016
  2. 01/22 git-compat-util.h: add the FORMATPRINTF macroElia Pinto, Feb 11, 2016
  3. 02/22 advice.h: use the FORMATPRINTF macro to declare the gcc function attribute 'format printf'Elia Pinto, Feb 11, 2016
  4. 03/22 argv-array.h: use the FORMATPRINTF macro to declare the gcc function attribute 'format printf'Elia Pinto, Feb 11, 2016
  5. Junio C HamanoFeb 11, 2016
  6. Junio C HamanoFeb 11, 2016
  7. 04/22 builtin/index-pack.c: use the FORMATPRINTF macro to declare the gcc function attribute 'format printf'Elia Pinto, Feb 11, 2016
  8. 05/22 builtin/receive-pack.c: use the FORMATPRINTF macro to declare the gcc function attribute 'format printf'Elia Pinto, Feb 11, 2016
  9. 06/22 builtin/update-index.c: use the FORMATPRINTF macro to declare the gcc function attribute 'format printf'Elia Pinto, Feb 11, 2016
  10. 07/22 builtin/upload-archive.c: use the FORMATPRINTF macro to declare the gcc function attribute 'format printf'Elia Pinto, Feb 11, 2016
  11. 08/22 cache.h: use the FORMATPRINTF macro to declare the gcc function attribute 'format printf'Elia Pinto, Feb 11, 2016
  12. 09/22 color.h: use the FORMATPRINTF macro to declare the gcc function attribute 'format printf'Elia Pinto, Feb 11, 2016
  13. 10/22 config.c: use the FORMATPRINTF macro to declare the gcc function attribute 'format printf'Elia Pinto, Feb 11, 2016
  14. 11/22 daemon.c: use the FORMATPRINTF macro to declare the gcc function attribute 'format printf'Elia Pinto, Feb 11, 2016
  15. 12/22 fsck.c: use the FORMATPRINTF macro to declare the gcc function attribute 'format printf'Elia Pinto, Feb 11, 2016
  16. 13/22 http-backend.c: use the FORMATPRINTF macro to declare the gcc function attribute 'format printf'Elia Pinto, Feb 11, 2016
  17. 14/22 imap-send.c: use the FORMATPRINTF macro to declare the gcc function attribute 'format printf'Elia Pinto, Feb 11, 2016
  18. 15/22 merge-recursive.c: use the FORMATPRINTF macro to declare the gcc function attribute 'format printf'Elia Pinto, Feb 11, 2016
  19. 16/22 pkt-line.h: use the FORMATPRINTF macro to declare the gcc function attribute 'format printf'Elia Pinto, Feb 11, 2016
  20. 17/22 remote.c: use the FORMATPRINTF macro to declare the gcc function attribute 'format printf'Elia Pinto, Feb 11, 2016
  21. 18/22 strbuf.h: use the FORMATPRINTF macro to declare the gcc function attribute 'format printf'Elia Pinto, Feb 11, 2016
  22. 19/22 trace.h: use the FORMATPRINTF macro to declare the gcc function attribute 'format printf'Elia Pinto, Feb 11, 2016
  23. 20/22 transport-helper.c: use the FORMATPRINTF macro to declare the gcc function attribute 'format printf'Elia Pinto, Feb 11, 2016
  24. 21/22 utf8.h: use the FORMATPRINTF macro to declare the gcc function attribute 'format printf'Elia Pinto, Feb 11, 2016
  25. 22/22 wt-status.h: use the FORMATPRINTF macro to declare the gcc function attribute 'format printf'Elia Pinto, Feb 11, 2016
  26. Junio C HamanoFeb 11, 2016
  27. Jeff KingFeb 11, 2016
  28. Junio C HamanoFeb 18, 2016

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.