On Mon, Jul 30, 2018 at 08:25:16PM +0200, SZEDER Gábor wrote:
> > diff --git a/sequencer.c b/sequencer.c
> > index 1c035ceec7..d257903db0 100644
> > --- a/sequencer.c
> > +++ b/sequencer.c
>
> > +int write_basic_state(struct replay_opts *opts, const char *head_name,
> > + const char *onto, const char *orig_head)
> > +{
> > + const char *quiet = getenv("GIT_QUIET");
> > +
> > + if (head_name)
> > + write_file(rebase_path_head_name(), "%s\n", head_name);
> > + if (onto)
> > + write_file(rebase_path_onto(), "%s\n", onto);
> > + if (orig_head)
> > + write_file(rebase_path_orig_head(), "%s\n", orig_head);
> > +
> > + if (quiet)
> > + write_file(rebase_path_quiet(), "%s\n", quiet);
> > + else
> > + write_file(rebase_path_quiet(), "");
>
> This is not a faithful conversion of the original. git-rebase.sh writes
> this 'quiet' file with:
>
> echo "$GIT_QUIET" > "$state_dir"/quiet
>
> which means that a single newline character was written even when
> $GIT_QUIET was unset/empty.
write_file() will call strbuf_complete_line(), so even passing "" will
result in a file with a newline in it (I didn't dig, but this comes from
e7ffa38c in 2015, so it may well have been a response to the breakage
you were thinking of).
So actually all of the "%s\n" here can be just "%s".
But there _is_ a reason not to use "", which is that it triggers
-Wformat-zero-length (which is part of -Wall unless you explicitly turn
it off, which our DEVELOPER=1 setup does). For a long time you _had_ to
do that, but these days we're actually clean with respect to that
warning.
So using "\n" here is better, and likewise here:
> > + if (opts->verbose)
> > + write_file(rebase_path_verbose(), "");
Unless we really do want a zero-length file, in which case:
write_file_buf(path, "", 0);
is the right option. That matches the shell version, but I'd be
surprised if it mattered, since it's clearly meant to be "if this file
exists, we're verbose".
-Peff
