Re: fatal error when diffing changed symlinks

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:

quoted

quoted

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