From 7daf4f2ac7b8fdddf6d8e128dc30e7daec7abad2 Mon Sep 17 00:00:00 2001 From: Ralf Thielow Date: Thu, 27 Feb 2020 20:25:30 +0000 Subject: [PATCH 1/2] rebase-interactive.c: silence format-zero-length warnings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes the following warnings: rebase-interactive.c: In function ‘edit_todo_list’: rebase-interactive.c:137:38: warning: zero-length gnu_printf format string [-Wformat-zero-length] write_file(rebase_path_dropped(), ""); rebase-interactive.c:144:37: warning: zero-length gnu_printf format string [-Wformat-zero-length] write_file(rebase_path_dropped(), ""); Signed-off-by: Ralf Thielow Signed-off-by: Junio C Hamano --- rebase-interactive.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rebase-interactive.c b/rebase-interactive.c index ac001dea58..0a4572e67e 100644 --- a/rebase-interactive.c +++ b/rebase-interactive.c @@ -134,14 +134,14 @@ int edit_todo_list(struct repository *r, struct todo_list *todo_list, if (incorrect) { if (todo_list_check_against_backup(r, new_todo)) { - write_file(rebase_path_dropped(), ""); + write_file(rebase_path_dropped(), "%s", ""); return -4; } if (incorrect > 0) unlink(rebase_path_dropped()); } else if (todo_list_check(todo_list, new_todo)) { - write_file(rebase_path_dropped(), ""); + write_file(rebase_path_dropped(), "%s", ""); return -4; } From 7329d94be72654f7f211e1bfa5ce3dd6fd2dc1fa Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 27 Feb 2020 18:54:45 -0500 Subject: [PATCH 2/2] config.mak.dev: re-enable -Wformat-zero-length We recently triggered some -Wformat-zero-length warnings in the code, but no developers noticed because we suppress that warning in builds with the DEVELOPER=1 Makefile knob set. But we _don't_ suppress them in a non-developer build (and they're part of -Wall). So even though non-developers probably aren't using -Werror, they see the annoying warnings when they build. We've had back and forth discussion over the years on whether this warning is useful or not. In most cases we've seen, it's not true that the call is a mistake, since we're using its side effects (like adding a newline status_printf_ln()) or writing an empty string to a destination which is handled by the function (as in write_file()). And so we end up working around it in the source by passing ("%s", ""). There's more discussion in the subthread starting at: https://lore.kernel.org/git/xmqqtwaod7ly.fsf@gitster.mtv.corp.google.com/ The short of it is that we probably can't just disable the warning for everybody because of portability issues. And ignoring it for developers puts us in the situation we're in now, where non-dev builds are annoyed. Since the workaround is both rarely needed and fairly straight-forward, let's just commit to doing it as necessary, and re-enable the warning. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- config.mak.dev | 1 - 1 file changed, 1 deletion(-) diff --git a/config.mak.dev b/config.mak.dev index bf1f3fcdee..89b218d11a 100644 --- a/config.mak.dev +++ b/config.mak.dev @@ -9,7 +9,6 @@ endif DEVELOPER_CFLAGS += -Wall DEVELOPER_CFLAGS += -Wdeclaration-after-statement DEVELOPER_CFLAGS += -Wformat-security -DEVELOPER_CFLAGS += -Wno-format-zero-length DEVELOPER_CFLAGS += -Wold-style-definition DEVELOPER_CFLAGS += -Woverflow DEVELOPER_CFLAGS += -Wpointer-arith