* [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
* [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 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
* 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
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
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 read-only IMAP folder(s) and NNTP newsgroup(s).