Re: fatal error when diffing changed symlinks

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

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