[PATCH 0/2] fix status_printf_ln calls "zero-length format" warnings

STALE3737d

5 messages, 3 authors, 2016-06-15 · open the first message on its own page

[PATCH 0/2] fix status_printf_ln calls "zero-length format" warnings

From: Benoit Pierre <hidden>
Date: 2016-06-15 23:00:15

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

[PATCH 1/2] status: allow NULL fmt for status_printf/status_vprintf_ln

From: Benoit Pierre <hidden>
Date: 2016-06-15 23:00:15

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 <redacted>
---
 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

[PATCH 2/2] fix status_printf_ln calls "zero-length format" warnings

From: Benoit Pierre <hidden>
Date: 2016-06-15 23:00:15

Those happens with "gcc -Wformat-zero-length".

Change empty strings to NULL now that it's allowed.

Signed-off-by: Benoit Pierre <redacted>
---
 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

Re: [PATCH 0/2] fix status_printf_ln calls "zero-length format" warnings

From: Jeff King <hidden>
Date: 2016-06-15 23:00:16

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

Re: [PATCH 1/2] status: allow NULL fmt for status_printf/status_vprintf_ln

From: Eric Sunshine <hidden>
Date: 2016-06-15 23:00:16

On Mon, Mar 10, 2014 at 3:27 PM, Benoit Pierre [off-list ref] wrote:

quoted hunk

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 <redacted>
---
 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

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help