Re: [PATCH 1/1] Exit on invalid diff status of diff_filepair

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

Eric Sunshine <sunshine@sunshineco.com> writes:

>>     git (main) $ cd ~/Repos/ECS50-3/hw3-skeleton-broken/
>>     hw3-skeleton-broken (main) $ ~/Build/git/git diff
>>     Segmentation fault (core dumped)
>>     hw3-skeleton-broken (main) $
>>
>> Let me know if you have any feedback or suggestions.
>
> Do you have a reproduction recipe which demonstrates the problem which
> your patch fixes? Including the recipe in the patch's commit message
> would help reviewers better understand the circumstances under which
> the crash occurs since the descriptions provided by both the original
> problem report[1] and the submitted patch[2] seem rather nebulous[3].
>
> More importantly, if you have a reproduction recipe, then it can be
> used as the basis for creating a test which should accompany the patch
> (and which should be added to one of the `t/t40xx-*.sh` files). We can
> help you convert the reproduction recipe into a test if desired.

Nicely put.

It is a bug _elsewhere_ in the code for the .status member to be
unassigned when the control reaches that point, so a patch to exit
after that happens is not all that interesting.  Instead of exiting
there, we would want to see a patch against the place where it
should have set the .status member but fails to do so.

Maybe with a corrupted repository, some of the blob objects we read
for comparison may fail to load and the function may be taking an
early return instead of complaining and dying upon unreadable blob
(I am not saying that is the only or even a likely case; just giving
an example situation for illustrate the point), in which case we
would want to fix _that_ code to complain and die properly.

Thanks.