From 4621085b7eb2f4cffe16d508988ff9b4a874b4ef Mon Sep 17 00:00:00 2001 From: Jeff King Date: Tue, 9 Jul 2013 20:18:40 -0400 Subject: add missing "format" function attributes For most of our functions that take printf-like formats, we use gcc's __attribute__((format)) to get compiler warnings when the functions are misused. Let's give a few more functions the same protection. In most cases, the annotations do not uncover any actual bugs; the only code change needed is that we passed a size_t to transfer_debug, which expected an int. Since we expect the passed-in value to be a relatively small buffer size (and cast a similar value to int directly below), we can just cast away the problem. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano diff --git a/advice.h b/advice.h index 94caa32..d4c1764 100644 --- a/advice.h +++ b/advice.h @@ -19,6 +19,7 @@ extern int advice_detached_head; extern int advice_set_upstream_failure; int git_default_advice_config(const char *var, const char *value); +__attribute__((format (printf, 1, 2))) void advise(const char *advice, ...); int error_resolve_conflict(const char *me); extern void NORETURN die_resolve_conflict(const char *me); diff --git a/trace.c b/trace.c index 5ec0e3b..3d744d1 100644 --- a/trace.c +++ b/trace.c @@ -75,6 +75,7 @@ static void trace_vprintf(const char *key, const char *fmt, va_list ap) strbuf_release(&buf); } +__attribute__((format (printf, 2, 3))) static void trace_printf_key(const char *key, const char *fmt, ...) { va_list ap; diff --git a/transport-helper.c b/transport-helper.c index 522d791..111336a 100644 --- a/transport-helper.c +++ b/transport-helper.c @@ -955,6 +955,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))) static void transfer_debug(const char *fmt, ...) { va_list args; @@ -1040,7 +1041,7 @@ static int udt_do_read(struct unidirectional_transfer *t) return -1; } else if (bytes == 0) { transfer_debug("%s EOF (with %i bytes in buffer)", - t->src_name, t->bufuse); + t->src_name, (int)t->bufuse); t->state = SSTATE_FLUSHING; } else if (bytes > 0) { t->bufuse += bytes; diff --git a/utf8.h b/utf8.h index 32a7bfb..65d0e42 100644 --- a/utf8.h +++ b/utf8.h @@ -10,6 +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))) int utf8_fprintf(FILE *, const char *, ...); void strbuf_add_wrapped_text(struct strbuf *buf, -- cgit v0.10.2-6-g49f6 From eccb614924c9067eeceffa503e4da3683f1c8b6b Mon Sep 17 00:00:00 2001 From: Jeff King Date: Tue, 9 Jul 2013 20:19:12 -0400 Subject: use "sentinel" function attribute for variadic lists This attribute can help gcc notice when callers forget to add a NULL sentinel to the end of the function. This is our first use of the sentinel attribute, but we shouldn't need to #ifdef for other compilers, as __attribute__ is already a no-op on non-gcc-compatible compilers. Suggested-by: Bert Wesarg More-Spots-Found-By: Matt Kraai Signed-off-by: Jeff King Signed-off-by: Junio C Hamano diff --git a/argv-array.h b/argv-array.h index 40248d4..e805748 100644 --- a/argv-array.h +++ b/argv-array.h @@ -15,6 +15,7 @@ void argv_array_init(struct argv_array *); void argv_array_push(struct argv_array *, const char *); __attribute__((format (printf,2,3))) void argv_array_pushf(struct argv_array *, const char *fmt, ...); +__attribute__((sentinel)) void argv_array_pushl(struct argv_array *, ...); void argv_array_pop(struct argv_array *); void argv_array_clear(struct argv_array *); diff --git a/builtin/revert.c b/builtin/revert.c index 0401fdb..b8b5174 100644 --- a/builtin/revert.c +++ b/builtin/revert.c @@ -54,6 +54,7 @@ static int option_parse_x(const struct option *opt, return 0; } +__attribute__((sentinel)) static void verify_opt_compatible(const char *me, const char *base_opt, ...) { const char *this_opt; @@ -70,6 +71,7 @@ static void verify_opt_compatible(const char *me, const char *base_opt, ...) die(_("%s: %s cannot be used with %s"), me, this_opt, base_opt); } +__attribute__((sentinel)) static void verify_opt_mutually_compatible(const char *me, ...) { const char *opt1, *opt2 = NULL; diff --git a/exec_cmd.h b/exec_cmd.h index e2b546b..307b55c 100644 --- a/exec_cmd.h +++ b/exec_cmd.h @@ -7,6 +7,7 @@ extern const char *git_exec_path(void); extern void setup_path(void); extern const char **prepare_git_cmd(const char **argv); extern int execv_git_cmd(const char **argv); /* NULL terminated */ +__attribute__((sentinel)) extern int execl_git_cmd(const char *cmd, ...); extern const char *system_path(const char *path); diff --git a/run-command.h b/run-command.h index 221ce33..0a47679 100644 --- a/run-command.h +++ b/run-command.h @@ -46,6 +46,7 @@ int finish_command(struct child_process *); int run_command(struct child_process *); extern char *find_hook(const char *name); +__attribute__((sentinel)) extern int run_hook(const char *index_file, const char *name, ...); #define RUN_COMMAND_NO_STDIN 1 -- cgit v0.10.2-6-g49f6 From 8dd0ee823f1829a3aa228c3c73e31de5c89b5317 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Tue, 9 Jul 2013 20:23:28 -0400 Subject: wt-status: use "format" function attribute for status_printf These functions could benefit from the added compile-time safety of having the compiler check printf arguments. Unfortunately, we also sometimes pass an empty format string, which will cause false positives with -Wformat-zero-length. In this case, that warning is wrong because our function is not a no-op with an empty format: it may be printing colorized output along with a trailing newline. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano diff --git a/wt-status.h b/wt-status.h index 4121bc2..fb7152e 100644 --- a/wt-status.h +++ b/wt-status.h @@ -96,9 +96,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); -void status_printf_ln(struct wt_status *s, const char *color, const char *fmt, ...) - ; -void status_printf(struct wt_status *s, const char *color, const char *fmt, ...) - ; +__attribute__((format (printf, 3, 4))) +void status_printf_ln(struct wt_status *s, const char *color, const char *fmt, ...); +__attribute__((format (printf, 3, 4))) +void status_printf(struct wt_status *s, const char *color, const char *fmt, ...); #endif /* STATUS_H */ -- cgit v0.10.2-6-g49f6 From 9fe3edc47f1f17a53272671c572c90ba71eb4f74 Mon Sep 17 00:00:00 2001 From: Ramsay Jones Date: Thu, 18 Jul 2013 21:02:12 +0100 Subject: Add the LAST_ARG_MUST_BE_NULL macro The sentinel function attribute is not understood by versions of the gcc compiler prior to v4.0. At present, for earlier versions of gcc, the build issues 108 warnings related to the unknown attribute. In order to suppress the warnings, we conditionally define the LAST_ARG_MUST_BE_NULL macro to provide the sentinel attribute for gcc v4.0 and newer. Signed-off-by: Ramsay Jones Signed-off-by: Junio C Hamano diff --git a/argv-array.h b/argv-array.h index e805748..85ba438 100644 --- a/argv-array.h +++ b/argv-array.h @@ -15,7 +15,7 @@ void argv_array_init(struct argv_array *); void argv_array_push(struct argv_array *, const char *); __attribute__((format (printf,2,3))) void argv_array_pushf(struct argv_array *, const char *fmt, ...); -__attribute__((sentinel)) +LAST_ARG_MUST_BE_NULL void argv_array_pushl(struct argv_array *, ...); void argv_array_pop(struct argv_array *); void argv_array_clear(struct argv_array *); diff --git a/builtin/revert.c b/builtin/revert.c index b8b5174..1d2648b 100644 --- a/builtin/revert.c +++ b/builtin/revert.c @@ -54,7 +54,7 @@ static int option_parse_x(const struct option *opt, return 0; } -__attribute__((sentinel)) +LAST_ARG_MUST_BE_NULL static void verify_opt_compatible(const char *me, const char *base_opt, ...) { const char *this_opt; @@ -71,7 +71,7 @@ static void verify_opt_compatible(const char *me, const char *base_opt, ...) die(_("%s: %s cannot be used with %s"), me, this_opt, base_opt); } -__attribute__((sentinel)) +LAST_ARG_MUST_BE_NULL static void verify_opt_mutually_compatible(const char *me, ...) { const char *opt1, *opt2 = NULL; diff --git a/exec_cmd.h b/exec_cmd.h index 307b55c..e4c9702 100644 --- a/exec_cmd.h +++ b/exec_cmd.h @@ -7,7 +7,7 @@ extern const char *git_exec_path(void); extern void setup_path(void); extern const char **prepare_git_cmd(const char **argv); extern int execv_git_cmd(const char **argv); /* NULL terminated */ -__attribute__((sentinel)) +LAST_ARG_MUST_BE_NULL extern int execl_git_cmd(const char *cmd, ...); extern const char *system_path(const char *path); diff --git a/git-compat-util.h b/git-compat-util.h index e955bb5..10e3ba6 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -295,6 +295,13 @@ extern char *gitbasename(char *); #endif #endif +/* The sentinel attribute is valid from gcc version 4.0 */ +#if defined(__GNUC__) && (__GNUC__ >= 4) +#define LAST_ARG_MUST_BE_NULL __attribute__((sentinel)) +#else +#define LAST_ARG_MUST_BE_NULL +#endif + #include "compat/bswap.h" #ifdef USE_WILDMATCH diff --git a/run-command.h b/run-command.h index 0a47679..6b985af 100644 --- a/run-command.h +++ b/run-command.h @@ -46,7 +46,7 @@ int finish_command(struct child_process *); int run_command(struct child_process *); extern char *find_hook(const char *name); -__attribute__((sentinel)) +LAST_ARG_MUST_BE_NULL extern int run_hook(const char *index_file, const char *name, ...); #define RUN_COMMAND_NO_STDIN 1 -- cgit v0.10.2-6-g49f6