Re: [PATCH] diff: handle NULL meta-info when spawning external diff

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

Jeff King <peff@peff.net> writes:

>> $ git diff --no-index foo bar
>> zsh: segmentation fault (core dumped)  git diff --no-index foo bar
>
> Thanks for providing a simple reproduction recipe. There's a pretty
> straight-forward fix below, though it leaves open some question of
> whether there's another bug lurking with --no-index (but either way, I
> think we'd want this simple fix as a first step).

Yup, I agree with you that the "--no-index" mode violates the basic
design that "the other path" and "xfrm_msg" go hand-in-hand.  In its
two tree comparison mode "git diff --no-index A/ B/", it should be
able to behave sensibly, but in its two files comparison mode to
compare plain regular files 'foo' and 'bar', there is nothing it can
do reasonably, I am afraid.  You could say that the change is
renaming 'foo' to create 'bar', and feed consistent data that is
aligned with that rename to external diff, which might be slightly
more logical than showing a change to 'foo' that has no rename
involved (i.e. omitting "other name"), but neither is satisfying.

> But I'm not sure what fallout we might have from changing that behavior
> now. So this patch takes the less-risky option, and simply teaches
> run_external_diff() to avoid passing xfrm_msg when it's NULL. That makes
> it agnostic to whether "other" and "xfrm_msg" always come as a pair. It
> fixes the segfault now, and if we want to change the --no-index "other"
> behavior on top, it will handle that, too.

Sounds sensible.

Thanks.  Will queue.