* [PATCH 0/2] fix status_printf_ln calls "zero-length format" warnings
@ 2014-03-10 19:27 Benoit Pierre
2014-03-10 19:27 ` [PATCH 1/2] status: allow NULL fmt for status_printf/status_vprintf_ln Benoit Pierre
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Benoit Pierre @ 2014-03-10 19:27 UTC (permalink / raw)
To: git
Those happens with "gcc -Wformat-zero-length". Since passing NULL does not
generate a warning (as __attribute__((printf())) does not imply nonull), modify
status_printf/status_printf_ln to allow a NULL format and update calls with an
empty string.
Benoit Pierre (2):
status: allow NULL fmt for status_printf/status_vprintf_ln
fix status_printf_ln calls "zero-length format" warnings
builtin/commit.c | 2 +-
wt-status.c | 23 ++++++++++++-----------
2 files changed, 13 insertions(+), 12 deletions(-)
--
1.9.0
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH 1/2] status: allow NULL fmt for status_printf/status_vprintf_ln 2014-03-10 19:27 [PATCH 0/2] fix status_printf_ln calls "zero-length format" warnings Benoit Pierre @ 2014-03-10 19:27 ` Benoit Pierre 2014-03-10 20:13 ` Eric Sunshine 2014-03-10 19:27 ` [PATCH 2/2] fix status_printf_ln calls "zero-length format" warnings Benoit Pierre 2014-03-10 19:51 ` [PATCH 0/2] " Jeff King 2 siblings, 1 reply; 6+ messages in thread From: Benoit Pierre @ 2014-03-10 19:27 UTC (permalink / raw) To: git Useful for calling status_printf only to change/reset the color (and output an additional '\n' with status_vprintf_ln). Signed-off-by: Benoit Pierre <benoit.pierre@gmail.com> --- wt-status.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/wt-status.c b/wt-status.c index 4e55810..17f63a4 100644 --- a/wt-status.c +++ b/wt-status.c @@ -49,7 +49,8 @@ static void status_vprintf(struct wt_status *s, int at_bol, const char *color, struct strbuf linebuf = STRBUF_INIT; const char *line, *eol; - strbuf_vaddf(&sb, fmt, ap); + if (NULL != fmt) + strbuf_vaddf(&sb, fmt, ap); if (!sb.len) { if (s->display_comment_prefix) { strbuf_addch(&sb, comment_line_char); -- 1.9.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] status: allow NULL fmt for status_printf/status_vprintf_ln 2014-03-10 19:27 ` [PATCH 1/2] status: allow NULL fmt for status_printf/status_vprintf_ln Benoit Pierre @ 2014-03-10 20:13 ` Eric Sunshine 0 siblings, 0 replies; 6+ messages in thread From: Eric Sunshine @ 2014-03-10 20:13 UTC (permalink / raw) To: Benoit Pierre; +Cc: Git List On Mon, Mar 10, 2014 at 3:27 PM, Benoit Pierre <benoit.pierre@gmail.com> wrote: > Useful for calling status_printf only to change/reset the color (and > output an additional '\n' with status_vprintf_ln). > > Signed-off-by: Benoit Pierre <benoit.pierre@gmail.com> > --- > wt-status.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/wt-status.c b/wt-status.c > index 4e55810..17f63a4 100644 > --- a/wt-status.c > +++ b/wt-status.c > @@ -49,7 +49,8 @@ static void status_vprintf(struct wt_status *s, int at_bol, const char *color, > struct strbuf linebuf = STRBUF_INIT; > const char *line, *eol; > > - strbuf_vaddf(&sb, fmt, ap); > + if (NULL != fmt) In this codebase, if (fmt) would be more idiomatic. > + strbuf_vaddf(&sb, fmt, ap); > if (!sb.len) { > if (s->display_comment_prefix) { > strbuf_addch(&sb, comment_line_char); > -- > 1.9.0 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/2] fix status_printf_ln calls "zero-length format" warnings 2014-03-10 19:27 [PATCH 0/2] fix status_printf_ln calls "zero-length format" warnings Benoit Pierre 2014-03-10 19:27 ` [PATCH 1/2] status: allow NULL fmt for status_printf/status_vprintf_ln Benoit Pierre @ 2014-03-10 19:27 ` Benoit Pierre 2014-03-10 19:51 ` [PATCH 0/2] " Jeff King 2 siblings, 0 replies; 6+ messages in thread From: Benoit Pierre @ 2014-03-10 19:27 UTC (permalink / raw) To: git Those happens with "gcc -Wformat-zero-length". Change empty strings to NULL now that it's allowed. Signed-off-by: Benoit Pierre <benoit.pierre@gmail.com> --- builtin/commit.c | 2 +- wt-status.c | 20 ++++++++++---------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/builtin/commit.c b/builtin/commit.c index 3767478..6d19439 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -805,7 +805,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix, committer_ident.buf); if (ident_shown) - status_printf_ln(s, GIT_COLOR_NORMAL, ""); + status_printf_ln(s, GIT_COLOR_NORMAL, NULL); saved_color_setting = s->use_color; s->use_color = 0; diff --git a/wt-status.c b/wt-status.c index 17f63a4..d9b1d00 100644 --- a/wt-status.c +++ b/wt-status.c @@ -189,7 +189,7 @@ static void wt_status_print_unmerged_header(struct wt_status *s) } else { status_printf_ln(s, c, _(" (use \"git add/rm <file>...\" as appropriate to mark resolution)")); } - status_printf_ln(s, c, ""); + status_printf_ln(s, c, NULL); } static void wt_status_print_cached_header(struct wt_status *s) @@ -205,7 +205,7 @@ static void wt_status_print_cached_header(struct wt_status *s) status_printf_ln(s, c, _(" (use \"git reset %s <file>...\" to unstage)"), s->reference); else status_printf_ln(s, c, _(" (use \"git rm --cached <file>...\" to unstage)")); - status_printf_ln(s, c, ""); + status_printf_ln(s, c, NULL); } static void wt_status_print_dirty_header(struct wt_status *s, @@ -224,7 +224,7 @@ static void wt_status_print_dirty_header(struct wt_status *s, status_printf_ln(s, c, _(" (use \"git checkout -- <file>...\" to discard changes in working directory)")); if (has_dirty_submodules) status_printf_ln(s, c, _(" (commit or discard the untracked or modified content in submodules)")); - status_printf_ln(s, c, ""); + status_printf_ln(s, c, NULL); } static void wt_status_print_other_header(struct wt_status *s, @@ -236,12 +236,12 @@ static void wt_status_print_other_header(struct wt_status *s, if (!s->hints) return; status_printf_ln(s, c, _(" (use \"git %s <file>...\" to include in what will be committed)"), how); - status_printf_ln(s, c, ""); + status_printf_ln(s, c, NULL); } static void wt_status_print_trailer(struct wt_status *s) { - status_printf_ln(s, color(WT_STATUS_HEADER, s), ""); + status_printf_ln(s, color(WT_STATUS_HEADER, s), NULL); } #define quote_path quote_path_relative @@ -794,7 +794,7 @@ static void wt_status_print_other(struct wt_status *s, string_list_clear(&output, 0); strbuf_release(&buf); conclude: - status_printf_ln(s, GIT_COLOR_NORMAL, ""); + status_printf_ln(s, GIT_COLOR_NORMAL, NULL); } void wt_status_truncate_message_at_cut_line(struct strbuf *buf) @@ -1292,7 +1292,7 @@ void wt_status_print(struct wt_status *s) on_what = _("Not currently on any branch."); } } - status_printf(s, color(WT_STATUS_HEADER, s), ""); + status_printf(s, color(WT_STATUS_HEADER, s), NULL); status_printf_more(s, branch_status_color, "%s", on_what); status_printf_more(s, branch_color, "%s\n", branch_name); if (!s->is_initial) @@ -1305,9 +1305,9 @@ void wt_status_print(struct wt_status *s) free(state.detached_from); if (s->is_initial) { - status_printf_ln(s, color(WT_STATUS_HEADER, s), ""); + status_printf_ln(s, color(WT_STATUS_HEADER, s), NULL); status_printf_ln(s, color(WT_STATUS_HEADER, s), _("Initial commit")); - status_printf_ln(s, color(WT_STATUS_HEADER, s), ""); + status_printf_ln(s, color(WT_STATUS_HEADER, s), NULL); } wt_status_print_updated(s); @@ -1324,7 +1324,7 @@ void wt_status_print(struct wt_status *s) if (s->show_ignored_files) wt_status_print_other(s, &s->ignored, _("Ignored files"), "add -f"); if (advice_status_u_option && 2000 < s->untracked_in_ms) { - status_printf_ln(s, GIT_COLOR_NORMAL, ""); + status_printf_ln(s, GIT_COLOR_NORMAL, NULL); status_printf_ln(s, GIT_COLOR_NORMAL, _("It took %.2f seconds to enumerate untracked files. 'status -uno'\n" "may speed it up, but you have to be careful not to forget to add\n" -- 1.9.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 0/2] fix status_printf_ln calls "zero-length format" warnings 2014-03-10 19:27 [PATCH 0/2] fix status_printf_ln calls "zero-length format" warnings Benoit Pierre 2014-03-10 19:27 ` [PATCH 1/2] status: allow NULL fmt for status_printf/status_vprintf_ln Benoit Pierre 2014-03-10 19:27 ` [PATCH 2/2] fix status_printf_ln calls "zero-length format" warnings Benoit Pierre @ 2014-03-10 19:51 ` Jeff King 2014-03-11 19:25 ` Junio C Hamano 2 siblings, 1 reply; 6+ messages in thread From: Jeff King @ 2014-03-10 19:51 UTC (permalink / raw) To: Benoit Pierre; +Cc: git On Mon, Mar 10, 2014 at 08:27:25PM +0100, Benoit Pierre wrote: > Those happens with "gcc -Wformat-zero-length". Since passing NULL does not > generate a warning (as __attribute__((printf())) does not imply nonull), modify > status_printf/status_printf_ln to allow a NULL format and update calls with an > empty string. Most of us who compile with -Wall decided a while ago to use -Wno-format-zero-length, because it really is a silly complaint (it assumes there are no side effects of the function besides printing the format string, which is obviously not true in this case). It would be nice to compile out of the box with "-Wall -Werror", and I think your solution looks relatively clean. But I am slightly concerned about the assumption that it is OK to pass a NULL fmt parameter to a function marked with __attribute__((format)). It certainly seems to be the case now, and I do not know of any plans for that to change, but it seems like a potentially obvious thing for the compiler to check. I dunno; perhaps it has already been considered and rejected by gcc folks to allow the exact escape hatch we are using here. -Peff ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 0/2] fix status_printf_ln calls "zero-length format" warnings 2014-03-10 19:51 ` [PATCH 0/2] " Jeff King @ 2014-03-11 19:25 ` Junio C Hamano 0 siblings, 0 replies; 6+ messages in thread From: Junio C Hamano @ 2014-03-11 19:25 UTC (permalink / raw) To: Jeff King; +Cc: Benoit Pierre, git Jeff King <peff@peff.net> writes: > Most of us who compile with -Wall decided a while ago to use > -Wno-format-zero-length, because it really is a silly complaint (it > assumes there are no side effects of the function besides printing the > format string, which is obviously not true in this case). > > It would be nice to compile out of the box with "-Wall -Werror", and I > think your solution looks relatively clean. But I am slightly concerned > about the assumption that it is OK to pass a NULL fmt parameter to a > function marked with __attribute__((format)). It certainly seems to be > the case now, and I do not know of any plans for that to change, but it > seems like a potentially obvious thing for the compiler to check. Yes, exactly. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-03-11 19:25 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-03-10 19:27 [PATCH 0/2] fix status_printf_ln calls "zero-length format" warnings Benoit Pierre 2014-03-10 19:27 ` [PATCH 1/2] status: allow NULL fmt for status_printf/status_vprintf_ln Benoit Pierre 2014-03-10 20:13 ` Eric Sunshine 2014-03-10 19:27 ` [PATCH 2/2] fix status_printf_ln calls "zero-length format" warnings Benoit Pierre 2014-03-10 19:51 ` [PATCH 0/2] " Jeff King 2014-03-11 19:25 ` Junio C Hamano
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).