Re: [PATCH] sequencer: cleanup for gcc 8.2.1 warning

Junio C Hamano [off-list ref] writes:

I'd have to say that the ability to create an empty file is more
important in the longer term.  Can't the workaround be done to
write_file() instead?  I actually do not mind if the solution were
to introduce a newhelper "write_empty_file()", but the way it is
written in the code before this patch, i.e.

	write_file(FILENAME, "")

is so obvious a way to create an empty file, so if we do not have to
resort to such a hackery to special case an empty file, that would
be preferrable.

It turns out that we have dealt with this before.

The trick employed by 7d7d6802 ("silence a bunch of
format-zero-length warnings", 2014-05-04) is still a caller side
workaround, but to do

	-	status_printf_ln(s, GIT_COLOR_NORMAL, "");
	+	status_printf_ln(s, GIT_COLOR_NORMAL, "%s", "");

to the function whose third parameter is printf format.

I do not know if it is a good idea to define a macro

	#define	EMPTY_CONTENTS	"%s", ""

in git-compat-util.h and then replace all the occurrences of "%s", ""
in the source code with it.  That way, we'd be able to create an
empty file with

	write_file(FILENAME, EMPTY_CONTENTS);

and write out an empty line with

	status_printf_ln(s, GIT_COLOR_NORMAL, EMPTY_CONTENTS);

and they would read naturally.  But may be it is a bit too cute an
idea?  I dunno.


	

On Wed, Oct 24, 2018 at 11:22 PM Junio C Hamano [off-list ref] wrote:

and they would read naturally.  But may be it is a bit too cute an
idea?  I dunno.

my first idea was to replace it with a helper called touch_file, since
I was expecting it will be a popular operation as flag files are
common in shell scripts and that name will be second nature to anyone
porting them.

the fact that the quiet flag was created with a single '\n' in the
code just immediately above this make me go for the proposed
"solution" instead (which I verified wouldn't change behaviour as you
described in your post; I apologize for not documenting it in the
commit and wasting your time).

would something like this work better? (not to apply, and probably mangled)

--- a/sequencer.c
+++ b/sequencer.c

@@ -35,6 +35,8 @@

 #define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION"

+#define touch_file(path) write_file(path, "%s", "")
+
 const char sign_off_header[] = "Signed-off-by: ";
 static const char cherry_picked_prefix[] = "(cherry picked from commit ";

@@ -2389,7 +2391,7 @@ int write_basic_state(struct replay_opts *opts,

const char *head_name,
                write_file(rebase_path_quiet(), "\n");

        if (opts->verbose)
-               write_file(rebase_path_verbose(), "");
+               touch_file(rebase_path_verbose());
        if (opts->strategy)
                write_file(rebase_path_strategy(), "%s\n", opts->strategy);
        if (opts->xopts_nr > 0)