Re: [PATCH] diff: stop output garbled message in dry run mode

[Junio C Hamano <gitster@pobox.com>]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> I do see a still-unguarded `fprintf(o->file, ...)` call in
> `run_diff_cmd()`, but as far as I can see, this call is not in any code
> path where `dry_run` is set.

Among the callers of run_diff_cmd(), only the caller that wants to
report "this path is unmerged" passes NULL diff_filespec pointers in
parameters one and two, in which case run_diff_cmd() would give that
message.  So if you have an unmerged filepair in queued_diff, this
callchain

	diff_flush()
	  loop over diff_queued_diff
          -> diff_flush_patch_quietly()
	     fiddle with dry_run bit
	     -> diff_flush_patch()
		-> run_diff()
		   -> run_diff_cmd() with one&two set to NULL

may hit the fprintf into o->file.

So you are right to worry about that fprintf().  If I make a
whitespace-only change to one file, and then make another path
unmerged, here is what I would see:

    $ rungit v2.48.0 diff --raw
    :100644 100644 b82c4963e7 0000000000 M  cache-tree.h
    :000000 100644 0000000000 0000000000 U  t/lib-gpg.sh

This is version before that dry-run thing.  It operated under the
old rule to show "--raw" to report object differences, hence
ignoring "-w".

    $ rungit v2.48.0 diff --raw -w
    :100644 100644 b82c4963e7 0000000000 M  cache-tree.h
    :000000 100644 0000000000 0000000000 U  t/lib-gpg.sh

With a version with the dry_run thing, here is what we see:

    $ git diff --raw -w
    * Unmerged path t/lib-gpg.sh
    :000000 100644 0000000000 0000000000 U  t/lib-gpg.sh

As dry_run thing intended, the entry on the whitespace-only path is
gone from the output, but the fprintf(o->file) you noticed comes out,
which is not what we want to see.  Of course, if we omit -w to avoid
triggering the dry-run thing, we won't see it.

    $ git diff --raw
    :100644 100644 b82c4963e7 0000000000 M  cache-tree.h
    :000000 100644 0000000000 0000000000 U  t/lib-gpg.sh

As a regression-fix change, I'd feel safer with Peff's version.

Thanks.