On Fri, Feb 24, 2017 at 11:51:22AM -0800, Junio C Hamano wrote:
> > A slightly worse is that the upcoming Git will ship with a rewritten
> > "difftool" that makes the above sequence segfault.
>
> The culprit seems to be these lines in run_dir_diff():
>
> if (S_ISLNK(lmode)) {
> char *content = read_sha1_file(loid.hash, &type, &size);
> add_left_or_right(&symlinks2, src_path, content, 0);
> free(content);
> }
>
> if (S_ISLNK(rmode)) {
> char *content = read_sha1_file(roid.hash, &type, &size);
> add_left_or_right(&symlinks2, dst_path, content, 1);
> free(content);
> }
>
> When viewing a working tree file, oid.hash could be 0{40} and
> read_sha1_file() is not the right function to use to obtain the
> contents.
>
> Both of these two need to pay attention to 0{40}, I think, as the
> user may be running "difftool -R --dir-diff" in which case the
> working tree would appear in the left hand side instead.
As a side note, I think even outside of 0{40}, this should be checking
the return value of read_sha1_file(). A corrupted repo should die(), not
segfault.
-Peff