Hi Peff & Junio,
On Fri, 24 Feb 2017, Jeff King wrote:
> 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.
I agree. I am on it.
Ciao,
Dscho
