threads / patch / 41390

patch, 22 partsadd the FORMATPRINTF macro to declare the gcc function

Subject: [PATCH 00/22] add the FORMATPRINTF macro to declare the gcc function

## tl;dr

28 messages between Feb 11, 2016 and Feb 18, 2016. Diffs are folded; open one to read it.

replies: 27people: 3as markdown or json

Elia Pinto· Feb 11, 2016, 12:38 UTC · lore

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
  advice.h: use the FORMATPRINTF macro to declare the gcc function
    attribute 'format printf'
  argv-array.h: use the FORMATPRINTF macro to declare the gcc function
    attribute 'format printf'
  builtin/index-pack.c: use the FORMATPRINTF macro to declare the gcc
    function attribute 'format printf'
  builtin/receive-pack.c: use the FORMATPRINTF macro to declare the gcc
    function attribute 'format printf'
  builtin/update-index.c: use the FORMATPRINTF macro to declare the gcc
    function attribute 'format printf'
  builtin/upload-archive.c: use the FORMATPRINTF macro to declare the
    gcc function attribute 'format printf'
  cache.h: use the FORMATPRINTF macro to declare the gcc function
    attribute 'format printf'
  color.h: use the FORMATPRINTF macro to declare the gcc function
    attribute 'format printf'
  config.c: use the FORMATPRINTF macro to declare the gcc function
    attribute 'format printf'
  daemon.c: use the FORMATPRINTF macro to declare the gcc function
    attribute 'format printf'
  fsck.c: use the FORMATPRINTF macro to declare the gcc function
    attribute 'format printf'
  http-backend.c: use the FORMATPRINTF macro to declare the gcc function
    attribute 'format printf'
  imap-send.c: use the FORMATPRINTF macro to declare the gcc function
    attribute 'format printf'
  merge-recursive.c: use the FORMATPRINTF macro to declare the gcc
    function attribute 'format printf'
  pkt-line.h: use the FORMATPRINTF macro to declare the gcc function
    attribute 'format printf'
  remote.c: use the FORMATPRINTF macro to declare the gcc function
    attribute 'format printf'
  strbuf.h: use the FORMATPRINTF macro to declare the gcc function
    attribute 'format printf'
  trace.h: use the FORMATPRINTF macro to declare the gcc function
    attribute 'format printf'
  transport-helper.c: use the FORMATPRINTF macro to declare the gcc
    function attribute 'format printf'
  utf8.h: use the FORMATPRINTF macro to declare the gcc function
    attribute 'format printf'
  wt-status.h: use the FORMATPRINTF macro to declare the gcc function
    attribute 'format printf'
 advice.h                 |  3 +--
 argv-array.h             |  2 +-
 builtin/index-pack.c     |  2 +-
 builtin/receive-pack.c   |  4 ++--
 builtin/update-index.c   |  2 +-
 builtin/upload-archive.c |  2 +-
 cache.h                  | 22 +++++++++++-----------
 color.h                  |  4 ++--
 config.c                 |  2 +-
 daemon.c                 |  4 ++--
 fsck.c                   |  2 +-
 git-compat-util.h        | 14 ++++++++------
 http-backend.c           |  6 +++---
 imap-send.c              | 10 +++++-----
 merge-recursive.c        |  2 +-
 pkt-line.h               |  4 ++--
 remote.c                 |  2 +-
 strbuf.h                 | 14 +++++++-------
 trace.h                  | 16 ++++++++--------
 transport-helper.c       |  2 +-
 utf8.h                   |  2 +-
 wt-status.h              |  4 ++--
 22 files changed, 63 insertions(+), 62 deletions(-)
-- 
2.5.0
Elia Pinto· Feb 11, 2016, 12:38 UTC · re: Elia Pinto · lore

[PATCH 01/22] git-compat-util.h: add the FORMATPRINTF macro

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.

Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
 git-compat-util.h | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)
Show changes to git-compat-util.h +8 −6
diff --git a/git-compat-util.h b/git-compat-util.h
index 693a336..17349d9 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -388,6 +388,8 @@ static inline char *git_find_last_dir_sep(const char *path)
 #endif
 #endif
 
+#define FORMATPRINTF(a,b) __attribute__((format (printf, a, b)))
+
 /* The sentinel attribute is valid from gcc version 4.0 */
 #if defined(__GNUC__) && (__GNUC__ >= 4)
 #define LAST_ARG_MUST_BE_NULL __attribute__((sentinel))
@@ -404,11 +406,11 @@ struct strbuf;
 /* General helper functions */
 extern void vreportf(const char *prefix, const char *err, va_list params);
 extern NORETURN void usage(const char *err);
-extern NORETURN void usagef(const char *err, ...) __attribute__((format (printf, 1, 2)));
-extern NORETURN void die(const char *err, ...) __attribute__((format (printf, 1, 2)));
-extern NORETURN void die_errno(const char *err, ...) __attribute__((format (printf, 1, 2)));
-extern int error(const char *err, ...) __attribute__((format (printf, 1, 2)));
-extern void warning(const char *err, ...) __attribute__((format (printf, 1, 2)));
+extern NORETURN void usagef(const char *err, ...) FORMATPRINTF(1,2);
+extern NORETURN void die(const char *err, ...) FORMATPRINTF(1,2);
+extern NORETURN void die_errno(const char *err, ...) FORMATPRINTF(1,2);
+extern int error(const char *err, ...) FORMATPRINTF(1,2);
+extern void warning(const char *err, ...) FORMATPRINTF(1,2);
 
 #ifndef NO_OPENSSL
 #ifdef APPLE_COMMON_CRYPTO
@@ -759,7 +761,7 @@ static inline size_t xsize_t(off_t len)
 	return (size_t)len;
 }
 
-__attribute__((format (printf, 3, 4)))
+FORMATPRINTF(3,4)
 extern int xsnprintf(char *dst, size_t max, const char *fmt, ...);
 
 /* in ctype.c, for kwset users */

-- 
2.5.0
Elia Pinto· Feb 11, 2016, 12:38 UTC · re: Elia Pinto · lore

[PATCH 02/22] advice.h: use the FORMATPRINTF macro to declare the gcc function attribute 'format printf'

Use the new FORMATPRINTF macro (in git-compat-util.h) to declare the gcc function attribute 'format printf'

Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
 advice.h | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
Show changes to advice.h +1 −2
diff --git a/advice.h b/advice.h
index b341a55..eef82e1 100644
--- a/advice.h
+++ b/advice.h
@@ -20,8 +20,7 @@ extern int advice_object_name_warning;
 extern int advice_rm_hints;
 
 int git_default_advice_config(const char *var, const char *value);
-__attribute__((format (printf, 1, 2)))
-void advise(const char *advice, ...);
+FORMATPRINTF(1,2) void advise(const char *advice, ...);
 int error_resolve_conflict(const char *me);
 extern void NORETURN die_resolve_conflict(const char *me);
 void NORETURN die_conclude_merge(void);

-- 
2.5.0
Elia Pinto· Feb 11, 2016, 12:38 UTC · re: Elia Pinto · lore

[PATCH 03/22] argv-array.h: use the FORMATPRINTF macro to declare the gcc function attribute 'format printf'

Use the new FORMATPRINTF macro (in git-compat-util.h) to declare the gcc function attribute 'format printf'

Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
 argv-array.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Show changes to argv-array.h +1 −1
diff --git a/argv-array.h b/argv-array.h
index a2fa0aa..e303dfd 100644
--- a/argv-array.h
+++ b/argv-array.h
@@ -13,7 +13,7 @@ struct argv_array {
 
 void argv_array_init(struct argv_array *);
 void argv_array_push(struct argv_array *, const char *);
-__attribute__((format (printf,2,3)))
+FORMATPRINTF(2,3)
 void argv_array_pushf(struct argv_array *, const char *fmt, ...);
 LAST_ARG_MUST_BE_NULL
 void argv_array_pushl(struct argv_array *, ...);

-- 
2.5.0
Junio C Hamano· Feb 11, 2016, 20:53 UTC · re: Elia Pinto · lore

Re: [PATCH 03/22] argv-array.h: use the FORMATPRINTF macro to declare the gcc function attribute 'format printf'

Elia Pinto <gitter.spiros@gmail.com> writes:
Show 21 quoted lines
> Use the new FORMATPRINTF macro (in git-compat-util.h) to declare the gcc function
> attribute 'format printf'
>
> Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
> ---
>  argv-array.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/argv-array.h b/argv-array.h
> index a2fa0aa..e303dfd 100644
> --- a/argv-array.h
> +++ b/argv-array.h
> @@ -13,7 +13,7 @@ struct argv_array {
>  
>  void argv_array_init(struct argv_array *);
>  void argv_array_push(struct argv_array *, const char *);
> -__attribute__((format (printf,2,3)))
> +FORMATPRINTF(2,3)
>  void argv_array_pushf(struct argv_array *, const char *fmt, ...);
>  LAST_ARG_MUST_BE_NULL
>  void argv_array_pushl(struct argv_array *, ...);
OK, this may answer my previous question.

It might be pleasing to the eyes when these two macros appearing together in all CAPS, making them look similar and consistent, perhaps with two more tweaks:

 - Imitate LAST_ARG_MUST_BE_NULL and spell it FORMAT_PRINTF;
 - Consistently have SP after comma, i.e. FORMAT_PRINTF(2, 3),
   or to make it obvious that this thing is a special magic, drop SP
   (i.e. this patch would stay the same but some other patches left
   a space after comma, which made them look more inconsistent).
Thanks.  I am not 100% sold on this yet, though.
Junio C Hamano· Feb 11, 2016, 21:49 UTC · re: Junio C Hamano · lore

Re: [PATCH 03/22] argv-array.h: use the FORMATPRINTF macro to declare the gcc function attribute 'format printf'

Junio C Hamano <gitster@pobox.com> writes:
Show 14 quoted lines
> OK, this may answer my previous question.
>
> It might be pleasing to the eyes when these two macros appearing
> together in all CAPS, making them look similar and consistent,
> perhaps with two more tweaks:
>
>  - Imitate LAST_ARG_MUST_BE_NULL and spell it FORMAT_PRINTF;
>
>  - Consistently have SP after comma, i.e. FORMAT_PRINTF(2, 3),
>    or to make it obvious that this thing is a special magic, drop SP
>    (i.e. this patch would stay the same but some other patches left
>    a space after comma, which made them look more inconsistent).
>
> Thanks.  I am not 100% sold on this yet, though.

Regarding the second one, I misread the patch series. You consistently drop space and make the result FORMATPRINTF(X,Y).

I was fooled by the inconsistent original text, some of them using __attribute__((format (printf, X, Y))) while others saying __attribute__((format(printf,X,Y)) and some other variants.

Elia Pinto· Feb 11, 2016, 12:38 UTC · re: Elia Pinto · lore

[PATCH 04/22] builtin/index-pack.c: use the FORMATPRINTF macro to declare the gcc function attribute 'format printf'

Use the new FORMATPRINTF macro (in git-compat-util.h) to declare the gcc function attribute 'format printf'

Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
 builtin/index-pack.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Show changes to builtin/index-pack.c +1 −1
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 6a01509..5941963 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -339,7 +339,7 @@ static void parse_pack_header(void)
 }
 
 static NORETURN void bad_object(unsigned long offset, const char *format,
-		       ...) __attribute__((format (printf, 2, 3)));
+		       ...) FORMATPRINTF(2,3);
 
 static NORETURN void bad_object(unsigned long offset, const char *format, ...)
 {

-- 
2.5.0
Elia Pinto· Feb 11, 2016, 12:38 UTC · re: Elia Pinto · lore

[PATCH 05/22] builtin/receive-pack.c: use the FORMATPRINTF macro to declare the gcc function attribute 'format printf'

Use the new FORMATPRINTF macro (in git-compat-util.h) to declare the gcc function attribute 'format printf'

Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
 builtin/receive-pack.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
Show changes to builtin/receive-pack.c +2 −2
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index f2d6761..956278f 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -277,8 +277,8 @@ struct command {
 	char ref_name[FLEX_ARRAY]; /* more */
 };
 
-static void rp_error(const char *err, ...) __attribute__((format (printf, 1, 2)));
-static void rp_warning(const char *err, ...) __attribute__((format (printf, 1, 2)));
+static void rp_error(const char *err, ...) FORMATPRINTF(1,2);
+static void rp_warning(const char *err, ...) FORMATPRINTF(1,2);
 
 static void report_message(const char *prefix, const char *err, va_list params)
 {

-- 
2.5.0
Elia Pinto· Feb 11, 2016, 12:38 UTC · re: Elia Pinto · lore

[PATCH 06/22] builtin/update-index.c: use the FORMATPRINTF macro to declare the gcc function attribute 'format printf'

Use the new FORMATPRINTF macro (in git-compat-util.h) to declare the gcc function attribute 'format printf'

Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
 builtin/update-index.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Show changes to builtin/update-index.c +1 −1
diff --git a/builtin/update-index.c b/builtin/update-index.c
index 7c5c143..cc62b76 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -35,7 +35,7 @@ static int mark_skip_worktree_only;
 #define UNMARK_FLAG 2
 static struct strbuf mtime_dir = STRBUF_INIT;
 
-__attribute__((format (printf, 1, 2)))
+FORMATPRINTF(1,2)
 static void report(const char *fmt, ...)
 {
 	va_list vp;

-- 
2.5.0
Elia Pinto· Feb 11, 2016, 12:38 UTC · re: Elia Pinto · lore

[PATCH 07/22] builtin/upload-archive.c: use the FORMATPRINTF macro to declare the gcc function attribute 'format printf'

Use the new FORMATPRINTF macro (in git-compat-util.h) to declare the gcc function attribute 'format printf'

Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
 builtin/upload-archive.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Show changes to builtin/upload-archive.c +1 −1
diff --git a/builtin/upload-archive.c b/builtin/upload-archive.c
index dbfe14f..3d23106 100644
--- a/builtin/upload-archive.c
+++ b/builtin/upload-archive.c
@@ -46,7 +46,7 @@ int cmd_upload_archive_writer(int argc, const char **argv, const char *prefix)
 	return write_archive(sent_argv.argc, sent_argv.argv, prefix, 0, NULL, 1);
 }
 
-__attribute__((format (printf, 1, 2)))
+FORMATPRINTF(1,2)
 static void error_clnt(const char *fmt, ...)
 {
 	struct strbuf buf = STRBUF_INIT;

-- 
2.5.0
Elia Pinto· Feb 11, 2016, 12:38 UTC · re: Elia Pinto · lore

[PATCH 08/22] cache.h: use the FORMATPRINTF macro to declare the gcc function attribute 'format printf'

Use the new FORMATPRINTF macro (in git-compat-util.h) to declare the gcc function attribute 'format printf'

Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
 cache.h | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)
Show changes to cache.h +11 −11
diff --git a/cache.h b/cache.h
index 553b04b..fb29154 100644
--- a/cache.h
+++ b/cache.h
@@ -763,24 +763,24 @@ extern int check_repository_format(void);
  * using the safer "dup" or "strbuf" formats below (in some cases, the
  * unsafe versions have already been removed).
  */
-extern const char *mkpath(const char *fmt, ...) __attribute__((format (printf, 1, 2)));
-extern const char *git_path(const char *fmt, ...) __attribute__((format (printf, 1, 2)));
+extern const char *mkpath(const char *fmt, ...) FORMATPRINTF(1,2);
+extern const char *git_path(const char *fmt, ...) FORMATPRINTF(1,2);
 
 extern char *mksnpath(char *buf, size_t n, const char *fmt, ...)
-	__attribute__((format (printf, 3, 4)));
+	FORMATPRINTF(3,4);
 extern void strbuf_git_path(struct strbuf *sb, const char *fmt, ...)
-	__attribute__((format (printf, 2, 3)));
+	FORMATPRINTF(2,3);
 extern char *git_path_buf(struct strbuf *buf, const char *fmt, ...)
-	__attribute__((format (printf, 2, 3)));
+	FORMATPRINTF(2,3);
 extern void strbuf_git_path_submodule(struct strbuf *sb, const char *path,
 				      const char *fmt, ...)
-	__attribute__((format (printf, 3, 4)));
+	FORMATPRINTF(3,4);
 extern char *git_pathdup(const char *fmt, ...)
-	__attribute__((format (printf, 1, 2)));
+	FORMATPRINTF(1,2);
 extern char *mkpathdup(const char *fmt, ...)
-	__attribute__((format (printf, 1, 2)));
+	FORMATPRINTF(1,2);
 extern char *git_pathdup_submodule(const char *path, const char *fmt, ...)
-	__attribute__((format (printf, 2, 3)));
+	FORMATPRINTF(2,3);
 
 extern void report_linked_checkout_garbage(void);
 
@@ -1630,7 +1630,7 @@ struct key_value_info {
 	int linenr;
 };
 
-extern NORETURN void git_die_config(const char *key, const char *err, ...) __attribute__((format(printf, 2, 3)));
+extern NORETURN void git_die_config(const char *key, const char *err, ...) FORMATPRINTF(2,3);
 extern NORETURN void git_die_config_linenr(const char *key, const char *filename, int linenr);
 
 extern int committer_ident_sufficiently_given(void);
@@ -1643,7 +1643,7 @@ extern const char *git_mailmap_blob;
 
 /* IO helper functions */
 extern void maybe_flush_or_die(FILE *, const char *);
-__attribute__((format (printf, 2, 3)))
+FORMATPRINTF(2,3)
 extern void fprintf_or_die(FILE *, const char *fmt, ...);
 
 #define COPY_READ_ERROR (-2)

-- 
2.5.0
Elia Pinto· Feb 11, 2016, 12:38 UTC · re: Elia Pinto · lore

[PATCH 09/22] color.h: use the FORMATPRINTF macro to declare the gcc function attribute 'format printf'

Use the new FORMATPRINTF macro (in git-compat-util.h) to declare the gcc function attribute 'format printf'

Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
 color.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
Show changes to color.h +2 −2
diff --git a/color.h b/color.h
index e155d13..8f763d7 100644
--- a/color.h
+++ b/color.h
@@ -86,9 +86,9 @@ int git_config_colorbool(const char *var, const char *value);
 int want_color(int var);
 int color_parse(const char *value, char *dst);
 int color_parse_mem(const char *value, int len, char *dst);
-__attribute__((format (printf, 3, 4)))
+FORMATPRINTF(3,4)
 int color_fprintf(FILE *fp, const char *color, const char *fmt, ...);
-__attribute__((format (printf, 3, 4)))
+FORMATPRINTF(3,4)
 int color_fprintf_ln(FILE *fp, const char *color, const char *fmt, ...);
 void color_print_strbuf(FILE *fp, const char *color, const struct strbuf *sb);
 

-- 
2.5.0
Elia Pinto· Feb 11, 2016, 12:38 UTC · re: Elia Pinto · lore

[PATCH 10/22] config.c: use the FORMATPRINTF macro to declare the gcc function attribute 'format printf'

Use the new FORMATPRINTF macro (in git-compat-util.h) to declare the gcc function attribute 'format printf'

Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
 config.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Show changes to config.c +1 −1
diff --git a/config.c b/config.c
index 86a5eb2..671bfd3 100644
--- a/config.c
+++ b/config.c
@@ -1604,7 +1604,7 @@ void git_die_config_linenr(const char *key, const char *filename, int linenr)
 		    key, filename, linenr);
 }
 
-NORETURN __attribute__((format(printf, 2, 3)))
+NORETURN FORMATPRINTF(2,3)
 void git_die_config(const char *key, const char *err, ...)
 {
 	const struct string_list *values;

-- 
2.5.0
Elia Pinto· Feb 11, 2016, 12:38 UTC · re: Elia Pinto · lore

[PATCH 11/22] daemon.c: use the FORMATPRINTF macro to declare the gcc function attribute 'format printf'

Use the new FORMATPRINTF macro (in git-compat-util.h) to declare the gcc function attribute 'format printf'

Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
 daemon.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
Show changes to daemon.c +2 −2
diff --git a/daemon.c b/daemon.c
index 46b411c..b4a9207 100644
--- a/daemon.c
+++ b/daemon.c
@@ -95,7 +95,7 @@ static void logreport(int priority, const char *err, va_list params)
 	}
 }
 
-__attribute__((format (printf, 1, 2)))
+FORMATPRINTF(1,2)
 static void logerror(const char *err, ...)
 {
 	va_list params;
@@ -104,7 +104,7 @@ static void logerror(const char *err, ...)
 	va_end(params);
 }
 
-__attribute__((format (printf, 1, 2)))
+FORMATPRINTF(1,2)
 static void loginfo(const char *err, ...)
 {
 	va_list params;

-- 
2.5.0
Elia Pinto· Feb 11, 2016, 12:38 UTC · re: Elia Pinto · lore

[PATCH 12/22] fsck.c: use the FORMATPRINTF macro to declare the gcc function attribute 'format printf'

Use the new FORMATPRINTF macro (in git-compat-util.h) to declare the gcc function attribute 'format printf'

Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
 fsck.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Show changes to fsck.c +1 −1
diff --git a/fsck.c b/fsck.c
index c637f66..953b8e4 100644
--- a/fsck.c
+++ b/fsck.c
@@ -264,7 +264,7 @@ static void append_msg_id(struct strbuf *sb, const char *msg_id)
 	strbuf_addstr(sb, ": ");
 }
 
-__attribute__((format (printf, 4, 5)))
+FORMATPRINTF(4,5)
 static int report(struct fsck_options *options, struct object *object,
 	enum fsck_msg_id id, const char *fmt, ...)
 {

-- 
2.5.0
Elia Pinto· Feb 11, 2016, 12:38 UTC · re: Elia Pinto · lore

[PATCH 13/22] http-backend.c: use the FORMATPRINTF macro to declare the gcc function attribute 'format printf'

Use the new FORMATPRINTF macro (in git-compat-util.h) to declare the gcc function attribute 'format printf'

Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
 http-backend.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
Show changes to http-backend.c +3 −3
diff --git a/http-backend.c b/http-backend.c
index 8870a26..c05e99f 100644
--- a/http-backend.c
+++ b/http-backend.c
@@ -58,7 +58,7 @@ static const char *get_parameter(const char *name)
 	return i ? i->util : NULL;
 }
 
-__attribute__((format (printf, 2, 3)))
+FORMATPRINTF(2,3)
 static void format_write(int fd, const char *fmt, ...)
 {
 	static char buffer[1024];
@@ -116,7 +116,7 @@ static void end_headers(void)
 	write_or_die(1, "\r\n", 2);
 }
 
-__attribute__((format (printf, 1, 2)))
+FORMATPRINTF(1,2)
 static NORETURN void not_found(const char *err, ...)
 {
 	va_list params;
@@ -132,7 +132,7 @@ static NORETURN void not_found(const char *err, ...)
 	exit(0);
 }
 
-__attribute__((format (printf, 1, 2)))
+FORMATPRINTF(1,2)
 static NORETURN void forbidden(const char *err, ...)
 {
 	va_list params;

-- 
2.5.0
Elia Pinto· Feb 11, 2016, 12:38 UTC · re: Elia Pinto · lore

[PATCH 14/22] imap-send.c: use the FORMATPRINTF macro to declare the gcc function attribute 'format printf'

Use the new FORMATPRINTF macro (in git-compat-util.h) to declare the gcc function attribute 'format printf'

Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
 imap-send.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)
Show changes to imap-send.c +5 −5
diff --git a/imap-send.c b/imap-send.c
index 4d3b773..6739a7c 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -59,14 +59,14 @@ static struct option imap_send_options[] = {
 #define DRV_BOX_BAD     -2
 #define DRV_STORE_BAD   -3
 
-__attribute__((format (printf, 1, 2)))
+FORMATPRINTF(1,2)
 static void imap_info(const char *, ...);
-__attribute__((format (printf, 1, 2)))
+FORMATPRINTF(1,2)
 static void imap_warn(const char *, ...);
 
 static char *next_arg(char **);
 
-__attribute__((format (printf, 3, 4)))
+FORMATPRINTF(3,4)
 static int nfsnprintf(char *buf, int blen, const char *fmt, ...);
 
 static int nfvasprintf(char **strp, const char *fmt, va_list ap)
@@ -578,7 +578,7 @@ static struct imap_cmd *issue_imap_cmd(struct imap_store *ctx,
 	return cmd;
 }
 
-__attribute__((format (printf, 3, 4)))
+FORMATPRINTF(3,4)
 static int imap_exec(struct imap_store *ctx, struct imap_cmd_cb *cb,
 		     const char *fmt, ...)
 {
@@ -594,7 +594,7 @@ static int imap_exec(struct imap_store *ctx, struct imap_cmd_cb *cb,
 	return get_cmd_result(ctx, cmdp);
 }
 
-__attribute__((format (printf, 3, 4)))
+FORMATPRINTF(3,4)
 static int imap_exec_m(struct imap_store *ctx, struct imap_cmd_cb *cb,
 		       const char *fmt, ...)
 {

-- 
2.5.0
Elia Pinto· Feb 11, 2016, 12:38 UTC · re: Elia Pinto · lore

[PATCH 15/22] merge-recursive.c: use the FORMATPRINTF macro to declare the gcc function attribute 'format printf'

Use the new FORMATPRINTF macro (in git-compat-util.h) to declare the gcc function attribute 'format printf'

Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
 merge-recursive.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Show changes to merge-recursive.c +1 −1
diff --git a/merge-recursive.c b/merge-recursive.c
index 8eabde2..891d909 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -156,7 +156,7 @@ static void flush_output(struct merge_options *o)
 	}
 }
 
-__attribute__((format (printf, 3, 4)))
+FORMATPRINTF(3,4)
 static void output(struct merge_options *o, int v, const char *fmt, ...)
 {
 	va_list ap;

-- 
2.5.0
Elia Pinto· Feb 11, 2016, 12:38 UTC · re: Elia Pinto · lore

[PATCH 16/22] pkt-line.h: use the FORMATPRINTF macro to declare the gcc function attribute 'format printf'

Use the new FORMATPRINTF macro (in git-compat-util.h) to declare the gcc function attribute 'format printf'

Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
 pkt-line.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
Show changes to pkt-line.h +2 −2
diff --git a/pkt-line.h b/pkt-line.h
index 3cb9d91..d3ba3be 100644
--- a/pkt-line.h
+++ b/pkt-line.h
@@ -20,9 +20,9 @@
  * side can't, we stay with pure read/write interfaces.
  */
 void packet_flush(int fd);
-void packet_write(int fd, const char *fmt, ...) __attribute__((format (printf, 2, 3)));
+void packet_write(int fd, const char *fmt, ...) FORMATPRINTF(2,3);
 void packet_buf_flush(struct strbuf *buf);
-void packet_buf_write(struct strbuf *buf, const char *fmt, ...) __attribute__((format (printf, 2, 3)));
+void packet_buf_write(struct strbuf *buf, const char *fmt, ...) FORMATPRINTF(2,3);
 
 /*
  * Read a packetized line into the buffer, which must be at least size bytes

-- 
2.5.0
Elia Pinto· Feb 11, 2016, 12:38 UTC · re: Elia Pinto · lore

[PATCH 17/22] remote.c: use the FORMATPRINTF macro to declare the gcc function attribute 'format printf'

Use the new FORMATPRINTF macro (in git-compat-util.h) to declare the gcc function attribute 'format printf'

Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
 remote.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Show changes to remote.c +1 −1
diff --git a/remote.c b/remote.c
index 35940a5..579cfcb 100644
--- a/remote.c
+++ b/remote.c
@@ -1672,7 +1672,7 @@ int branch_merge_matches(struct branch *branch,
 	return refname_match(branch->merge[i]->src, refname);
 }
 
-__attribute((format (printf,2,3)))
+FORMATPRINTF(2,3)
 static const char *error_buf(struct strbuf *err, const char *fmt, ...)
 {
 	if (err) {

-- 
2.5.0
Elia Pinto· Feb 11, 2016, 12:38 UTC · re: Elia Pinto · lore

[PATCH 18/22] strbuf.h: use the FORMATPRINTF macro to declare the gcc function attribute 'format printf'

Use the new FORMATPRINTF macro (in git-compat-util.h) to declare the gcc function attribute 'format printf'

Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
 strbuf.h | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)
Show changes to strbuf.h +7 −7
diff --git a/strbuf.h b/strbuf.h
index f72fd14..c887dab 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -331,17 +331,17 @@ extern void strbuf_humanise_bytes(struct strbuf *buf, off_t bytes);
 /**
  * Add a formatted string to the buffer.
  */
-__attribute__((format (printf,2,3)))
+FORMATPRINTF(2,3)
 extern void strbuf_addf(struct strbuf *sb, const char *fmt, ...);
 
 /**
  * Add a formatted string prepended by a comment character and a
  * blank to the buffer.
  */
-__attribute__((format (printf, 2, 3)))
+FORMATPRINTF(2,3)
 extern void strbuf_commented_addf(struct strbuf *sb, const char *fmt, ...);
 
-__attribute__((format (printf,2,0)))
+FORMATPRINTF(2,0)
 extern void strbuf_vaddf(struct strbuf *sb, const char *fmt, va_list ap);
 
 /**
@@ -556,9 +556,9 @@ extern int strbuf_check_branch_ref(struct strbuf *sb, const char *name);
 extern void strbuf_addstr_urlencode(struct strbuf *, const char *,
 				    int reserved);
 
-__attribute__((format (printf,1,2)))
+FORMATPRINTF(1,2)
 extern int printf_ln(const char *fmt, ...);
-__attribute__((format (printf,2,3)))
+FORMATPRINTF(2,3)
 extern int fprintf_ln(FILE *fp, const char *fmt, ...);
 
 char *xstrdup_tolower(const char *);
@@ -567,9 +567,9 @@ char *xstrdup_tolower(const char *);
  * Create a newly allocated string using printf format. You can do this easily
  * with a strbuf, but this provides a shortcut to save a few lines.
  */
-__attribute__((format (printf, 1, 0)))
+FORMATPRINTF(1,0)
 char *xstrvfmt(const char *fmt, va_list ap);
-__attribute__((format (printf, 1, 2)))
+FORMATPRINTF(1,2)
 char *xstrfmt(const char *fmt, ...);
 
 #endif /* STRBUF_H */

-- 
2.5.0
Elia Pinto· Feb 11, 2016, 12:38 UTC · re: Elia Pinto · lore

[PATCH 19/22] trace.h: use the FORMATPRINTF macro to declare the gcc function attribute 'format printf'

Use the new FORMATPRINTF macro (in git-compat-util.h) to declare the gcc function attribute 'format printf'

Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
 trace.h | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)
Show changes to trace.h +8 −8
diff --git a/trace.h b/trace.h
index 179b249..14df130 100644
--- a/trace.h
+++ b/trace.h
@@ -22,23 +22,23 @@ extern void trace_verbatim(struct trace_key *key, const void *buf, unsigned len)
 
 #ifndef HAVE_VARIADIC_MACROS
 
-__attribute__((format (printf, 1, 2)))
+FORMATPRINTF(1,2)
 extern void trace_printf(const char *format, ...);
 
-__attribute__((format (printf, 2, 3)))
+FORMATPRINTF(2,3)
 extern void trace_printf_key(struct trace_key *key, const char *format, ...);
 
-__attribute__((format (printf, 2, 3)))
+FORMATPRINTF(2,3)
 extern void trace_argv_printf(const char **argv, const char *format, ...);
 
 extern void trace_strbuf(struct trace_key *key, const struct strbuf *data);
 
 /* Prints elapsed time (in nanoseconds) if GIT_TRACE_PERFORMANCE is enabled. */
-__attribute__((format (printf, 2, 3)))
+FORMATPRINTF(2,3)
 extern void trace_performance(uint64_t nanos, const char *format, ...);
 
 /* Prints elapsed time since 'start' if GIT_TRACE_PERFORMANCE is enabled. */
-__attribute__((format (printf, 2, 3)))
+FORMATPRINTF(2,3)
 extern void trace_performance_since(uint64_t start, const char *format, ...);
 
 #else
@@ -97,15 +97,15 @@ extern void trace_performance_since(uint64_t start, const char *format, ...);
 			     __VA_ARGS__)
 
 /* backend functions, use non-*fl macros instead */
-__attribute__((format (printf, 4, 5)))
+FORMATPRINTF(4,5)
 extern void trace_printf_key_fl(const char *file, int line, struct trace_key *key,
 				const char *format, ...);
-__attribute__((format (printf, 4, 5)))
+FORMATPRINTF(4,5)
 extern void trace_argv_printf_fl(const char *file, int line, const char **argv,
 				 const char *format, ...);
 extern void trace_strbuf_fl(const char *file, int line, struct trace_key *key,
 			    const struct strbuf *data);
-__attribute__((format (printf, 4, 5)))
+FORMATPRINTF(4,5)
 extern void trace_performance_fl(const char *file, int line,
 				 uint64_t nanos, const char *fmt, ...);
 

-- 
2.5.0
Elia Pinto· Feb 11, 2016, 12:38 UTC · re: Elia Pinto · lore

[PATCH 20/22] transport-helper.c: use the FORMATPRINTF macro to declare the gcc function attribute 'format printf'

Use the new FORMATPRINTF macro (in git-compat-util.h) to declare the gcc function attribute 'format printf'

Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
 transport-helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Show changes to transport-helper.c +1 −1
diff --git a/transport-helper.c b/transport-helper.c
index a6bff8b..94c7aab 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -1069,7 +1069,7 @@ int transport_helper_init(struct transport *transport, const char *name)
 #define PBUFFERSIZE 8192
 
 /* Print bidirectional transfer loop debug message. */
-__attribute__((format (printf, 1, 2)))
+FORMATPRINTF(1,2)
 static void transfer_debug(const char *fmt, ...)
 {
 	va_list args;

-- 
2.5.0
Elia Pinto· Feb 11, 2016, 12:38 UTC · re: Elia Pinto · lore

[PATCH 21/22] utf8.h: use the FORMATPRINTF macro to declare the gcc function attribute 'format printf'

Use the new FORMATPRINTF macro (in git-compat-util.h) to declare the gcc function attribute 'format printf'

Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
 utf8.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Show changes to utf8.h +1 −1
diff --git a/utf8.h b/utf8.h
index 7930b44..3dc301c 100644
--- a/utf8.h
+++ b/utf8.h
@@ -10,7 +10,7 @@ int utf8_strwidth(const char *string);
 int is_utf8(const char *text);
 int is_encoding_utf8(const char *name);
 int same_encoding(const char *, const char *);
-__attribute__((format (printf, 2, 3)))
+FORMATPRINTF(2,3)
 int utf8_fprintf(FILE *, const char *, ...);
 
 extern const char utf8_bom[];

-- 
2.5.0
Elia Pinto· Feb 11, 2016, 12:38 UTC · re: Elia Pinto · lore

[PATCH 22/22] wt-status.h: use the FORMATPRINTF macro to declare the gcc function attribute 'format printf'

Use the new FORMATPRINTF macro (in git-compat-util.h) to declare the gcc function attribute 'format printf'

Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
 wt-status.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
Show changes to wt-status.h +2 −2
diff --git a/wt-status.h b/wt-status.h
index c9b3b74..f8b403a 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -104,9 +104,9 @@ void wt_status_get_state(struct wt_status_state *state, int get_detached_from);
 void wt_shortstatus_print(struct wt_status *s);
 void wt_porcelain_print(struct wt_status *s);
 
-__attribute__((format (printf, 3, 4)))
+FORMATPRINTF(3,4)
 void status_printf_ln(struct wt_status *s, const char *color, const char *fmt, ...);
-__attribute__((format (printf, 3, 4)))
+FORMATPRINTF(3,4)
 void status_printf(struct wt_status *s, const char *color, const char *fmt, ...);
 
 #endif /* STATUS_H */

-- 
2.5.0
Junio C Hamano· Feb 11, 2016, 17:59 UTC · re: Elia Pinto · lore

Re: [PATCH 00/22] add the FORMATPRINTF macro to declare the gcc function

Elia Pinto <gitter.spiros@gmail.com> writes:
Show 6 quoted lines
> 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?
Jeff King· Feb 11, 2016, 22:33 UTC · re: Junio C Hamano · lore

Re: [PATCH 00/22] add the FORMATPRINTF macro to declare the gcc function

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
Junio C Hamano· Feb 18, 2016, 21:59 UTC · re: Jeff King · lore

Re: [PATCH 00/22] add the FORMATPRINTF macro to declare the gcc function

Jeff King <peff@peff.net> writes:
Show 49 quoted lines
> On Thu, Feb 11, 2016 at 09:59:21AM -0800, Junio C Hamano wrote:
>
>> 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.
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.

← back to recent threads