{"thread":{"id":"36112","subject":"[PATCH 0/2] fix status_printf_ln calls \"zero-length format\" warnings","startedAt":"2014-03-10T19:27:25Z","lastAt":"2014-03-11T19:25:45Z","messageCount":6,"participants":["Benoit Pierre","Jeff King","Eric Sunshine","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"236439","messageId":"1394479647-22361-1-git-send-email-benoit.pierre@gmail.com","threadId":"36112","inReplyTo":null,"subject":"[PATCH 0/2] fix status_printf_ln calls \"zero-length format\" warnings","fromName":"Benoit Pierre","fromEmail":"benoit.pierre@gmail.com","sentAt":"2014-03-10T19:27:25Z","receivedAt":"2014-03-10T19:27:25Z","isPatch":true,"sender":{"key":"benoit.pierre@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5104286?v=4"},"body":"Those happens with \"gcc -Wformat-zero-length\". Since passing NULL does not\ngenerate a warning (as __attribute__((printf())) does not imply nonull), modify\nstatus_printf/status_printf_ln to allow a NULL format and update calls with an\nempty string.\n\nBenoit Pierre (2):\n status: allow NULL fmt for status_printf/status_vprintf_ln\n fix status_printf_ln calls \"zero-length format\" warnings\n\n builtin/commit.c | 2 +-\n wt-status.c | 23 ++++++++++++-----------\n 2 files changed, 13 insertions(+), 12 deletions(-)\n\n-- \n1.9.0\n"},{"id":"236440","messageId":"1394479647-22361-2-git-send-email-benoit.pierre@gmail.com","threadId":"36112","inReplyTo":"1394479647-22361-1-git-send-email-benoit.pierre@gmail.com","subject":"[PATCH 1/2] status: allow NULL fmt for status_printf/status_vprintf_ln","fromName":"Benoit Pierre","fromEmail":"benoit.pierre@gmail.com","sentAt":"2014-03-10T19:27:26Z","receivedAt":"2014-03-10T19:27:26Z","isPatch":true,"sender":{"key":"benoit.pierre@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5104286?v=4"},"body":"Useful for calling status_printf only to change/reset the color (and\noutput an additional '\\n' with status_vprintf_ln).\n\nSigned-off-by: Benoit Pierre \n---\n wt-status.c | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/wt-status.c b/wt-status.c\nindex 4e55810..17f63a4 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -49,7 +49,8 @@ static void status_vprintf(struct wt_status *s, int at_bol, const char *color,\n \tstruct strbuf linebuf = STRBUF_INIT;\n \tconst char *line, *eol;\n \n-\tstrbuf_vaddf(&sb, fmt, ap);\n+\tif (NULL != fmt)\n+\t strbuf_vaddf(&sb, fmt, ap);\n \tif (!sb.len) {\n \t\tif (s->display_comment_prefix) {\n \t\t\tstrbuf_addch(&sb, comment_line_char);\n-- \n1.9.0\n"},{"id":"236441","messageId":"1394479647-22361-3-git-send-email-benoit.pierre@gmail.com","threadId":"36112","inReplyTo":"1394479647-22361-1-git-send-email-benoit.pierre@gmail.com","subject":"[PATCH 2/2] fix status_printf_ln calls \"zero-length format\" warnings","fromName":"Benoit Pierre","fromEmail":"benoit.pierre@gmail.com","sentAt":"2014-03-10T19:27:27Z","receivedAt":"2014-03-10T19:27:27Z","isPatch":true,"sender":{"key":"benoit.pierre@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5104286?v=4"},"body":"Those happens with \"gcc -Wformat-zero-length\".\n\nChange empty strings to NULL now that it's allowed.\n\nSigned-off-by: Benoit Pierre \n---\n builtin/commit.c | 2 +-\n wt-status.c | 20 ++++++++++----------\n 2 files changed, 11 insertions(+), 11 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 3767478..6d19439 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -805,7 +805,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n \t\t\t\tcommitter_ident.buf);\n \n \t\tif (ident_shown)\n-\t\t\tstatus_printf_ln(s, GIT_COLOR_NORMAL, \"\");\n+\t\t\tstatus_printf_ln(s, GIT_COLOR_NORMAL, NULL);\n \n \t\tsaved_color_setting = s->use_color;\n \t\ts->use_color = 0;\ndiff --git a/wt-status.c b/wt-status.c\nindex 17f63a4..d9b1d00 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -189,7 +189,7 @@ static void wt_status_print_unmerged_header(struct wt_status *s)\n \t} else {\n \t\tstatus_printf_ln(s, c, _(\" (use \\\"git add/rm ...\\\" as appropriate to mark resolution)\"));\n \t}\n-\tstatus_printf_ln(s, c, \"\");\n+\tstatus_printf_ln(s, c, NULL);\n }\n \n static void wt_status_print_cached_header(struct wt_status *s)\n@@ -205,7 +205,7 @@ static void wt_status_print_cached_header(struct wt_status *s)\n \t\tstatus_printf_ln(s, c, _(\" (use \\\"git reset %s ...\\\" to unstage)\"), s->reference);\n \telse\n \t\tstatus_printf_ln(s, c, _(\" (use \\\"git rm --cached ...\\\" to unstage)\"));\n-\tstatus_printf_ln(s, c, \"\");\n+\tstatus_printf_ln(s, c, NULL);\n }\n \n static void wt_status_print_dirty_header(struct wt_status *s,\n@@ -224,7 +224,7 @@ static void wt_status_print_dirty_header(struct wt_status *s,\n \tstatus_printf_ln(s, c, _(\" (use \\\"git checkout -- ...\\\" to discard changes in working directory)\"));\n \tif (has_dirty_submodules)\n \t\tstatus_printf_ln(s, c, _(\" (commit or discard the untracked or modified content in submodules)\"));\n-\tstatus_printf_ln(s, c, \"\");\n+\tstatus_printf_ln(s, c, NULL);\n }\n \n static void wt_status_print_other_header(struct wt_status *s,\n@@ -236,12 +236,12 @@ static void wt_status_print_other_header(struct wt_status *s,\n \tif (!s->hints)\n \t\treturn;\n \tstatus_printf_ln(s, c, _(\" (use \\\"git %s ...\\\" to include in what will be committed)\"), how);\n-\tstatus_printf_ln(s, c, \"\");\n+\tstatus_printf_ln(s, c, NULL);\n }\n \n static void wt_status_print_trailer(struct wt_status *s)\n {\n-\tstatus_printf_ln(s, color(WT_STATUS_HEADER, s), \"\");\n+\tstatus_printf_ln(s, color(WT_STATUS_HEADER, s), NULL);\n }\n \n #define quote_path quote_path_relative\n@@ -794,7 +794,7 @@ static void wt_status_print_other(struct wt_status *s,\n \tstring_list_clear(&output, 0);\n \tstrbuf_release(&buf);\n conclude:\n-\tstatus_printf_ln(s, GIT_COLOR_NORMAL, \"\");\n+\tstatus_printf_ln(s, GIT_COLOR_NORMAL, NULL);\n }\n \n void wt_status_truncate_message_at_cut_line(struct strbuf *buf)\n@@ -1292,7 +1292,7 @@ void wt_status_print(struct wt_status *s)\n \t\t\t\ton_what = _(\"Not currently on any branch.\");\n \t\t\t}\n \t\t}\n-\t\tstatus_printf(s, color(WT_STATUS_HEADER, s), \"\");\n+\t\tstatus_printf(s, color(WT_STATUS_HEADER, s), NULL);\n \t\tstatus_printf_more(s, branch_status_color, \"%s\", on_what);\n \t\tstatus_printf_more(s, branch_color, \"%s\\n\", branch_name);\n \t\tif (!s->is_initial)\n@@ -1305,9 +1305,9 @@ void wt_status_print(struct wt_status *s)\n \tfree(state.detached_from);\n \n \tif (s->is_initial) {\n-\t\tstatus_printf_ln(s, color(WT_STATUS_HEADER, s), \"\");\n+\t\tstatus_printf_ln(s, color(WT_STATUS_HEADER, s), NULL);\n \t\tstatus_printf_ln(s, color(WT_STATUS_HEADER, s), _(\"Initial commit\"));\n-\t\tstatus_printf_ln(s, color(WT_STATUS_HEADER, s), \"\");\n+\t\tstatus_printf_ln(s, color(WT_STATUS_HEADER, s), NULL);\n \t}\n \n \twt_status_print_updated(s);\n@@ -1324,7 +1324,7 @@ void wt_status_print(struct wt_status *s)\n \t\tif (s->show_ignored_files)\n \t\t\twt_status_print_other(s, &s->ignored, _(\"Ignored files\"), \"add -f\");\n \t\tif (advice_status_u_option && 2000 < s->untracked_in_ms) {\n-\t\t\tstatus_printf_ln(s, GIT_COLOR_NORMAL, \"\");\n+\t\t\tstatus_printf_ln(s, GIT_COLOR_NORMAL, NULL);\n \t\t\tstatus_printf_ln(s, GIT_COLOR_NORMAL,\n \t\t\t\t\t _(\"It took %.2f seconds to enumerate untracked files. 'status -uno'\\n\"\n \t\t\t\t\t \"may speed it up, but you have to be careful not to forget to add\\n\"\n-- \n1.9.0\n"},{"id":"236445","messageId":"20140310195121.GB24568@sigill.intra.peff.net","threadId":"36112","inReplyTo":"1394479647-22361-1-git-send-email-benoit.pierre@gmail.com","subject":"Re: [PATCH 0/2] fix status_printf_ln calls \"zero-length format\" warnings","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-03-10T19:51:21Z","receivedAt":"2014-03-10T19:51:21Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Mar 10, 2014 at 08:27:25PM +0100, Benoit Pierre wrote:\n\n> Those happens with \"gcc -Wformat-zero-length\". Since passing NULL does not\n> generate a warning (as __attribute__((printf())) does not imply nonull), modify\n> status_printf/status_printf_ln to allow a NULL format and update calls with an\n> empty string.\n\nMost of us who compile with -Wall decided a while ago to use\n-Wno-format-zero-length, because it really is a silly complaint (it\nassumes there are no side effects of the function besides printing the\nformat string, which is obviously not true in this case).\n\nIt would be nice to compile out of the box with \"-Wall -Werror\", and I\nthink your solution looks relatively clean. But I am slightly concerned\nabout the assumption that it is OK to pass a NULL fmt parameter to a\nfunction marked with __attribute__((format)). It certainly seems to be\nthe case now, and I do not know of any plans for that to change, but it\nseems like a potentially obvious thing for the compiler to check.\n\nI dunno; perhaps it has already been considered and rejected by gcc\nfolks to allow the exact escape hatch we are using here.\n\n-Peff\n"},{"id":"236451","messageId":"CAPig+cTzVO1kJCM0zTiDza+Na4PHeLpAr1dvqsXxfbmpy-egHA@mail.gmail.com","threadId":"36112","inReplyTo":"1394479647-22361-2-git-send-email-benoit.pierre@gmail.com","subject":"Re: [PATCH 1/2] status: allow NULL fmt for status_printf/status_vprintf_ln","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2014-03-10T20:13:44Z","receivedAt":"2014-03-10T20:13:44Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Mar 10, 2014 at 3:27 PM, Benoit Pierre wrote:\n> Useful for calling status_printf only to change/reset the color (and\n> output an additional '\\n' with status_vprintf_ln).\n>\n> Signed-off-by: Benoit Pierre \n> ---\n> wt-status.c | 3 ++-\n> 1 file changed, 2 insertions(+), 1 deletion(-)\n>\n> diff --git a/wt-status.c b/wt-status.c\n> index 4e55810..17f63a4 100644\n> --- a/wt-status.c\n> +++ b/wt-status.c\n> @@ -49,7 +49,8 @@ static void status_vprintf(struct wt_status *s, int at_bol, const char *color,\n> struct strbuf linebuf = STRBUF_INIT;\n> const char *line, *eol;\n>\n> - strbuf_vaddf(&sb, fmt, ap);\n> + if (NULL != fmt)\n\nIn this codebase,\n\n if (fmt)\n\nwould be more idiomatic.\n\n> + strbuf_vaddf(&sb, fmt, ap);\n> if (!sb.len) {\n> if (s->display_comment_prefix) {\n> strbuf_addch(&sb, comment_line_char);\n> --\n> 1.9.0\n"},{"id":"236520","messageId":"xmqq38ior0zq.fsf@gitster.dls.corp.google.com","threadId":"36112","inReplyTo":"20140310195121.GB24568@sigill.intra.peff.net","subject":"Re: [PATCH 0/2] fix status_printf_ln calls \"zero-length format\" warnings","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-03-11T19:25:45Z","receivedAt":"2014-03-11T19:25:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> Most of us who compile with -Wall decided a while ago to use\n> -Wno-format-zero-length, because it really is a silly complaint (it\n> assumes there are no side effects of the function besides printing the\n> format string, which is obviously not true in this case).\n>\n> It would be nice to compile out of the box with \"-Wall -Werror\", and I\n> think your solution looks relatively clean. But I am slightly concerned\n> about the assumption that it is OK to pass a NULL fmt parameter to a\n> function marked with __attribute__((format)). It certainly seems to be\n> the case now, and I do not know of any plans for that to change, but it\n> seems like a potentially obvious thing for the compiler to check.\n\nYes, exactly.\n"}]}