Re: [PATCH] diff: fix lstat() error handling in diff_populate_filespec()

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 27, 2017, 16:35 UTC
Message-ID: <xmqq3764czv9.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <72e8146b-d4ab-ad7e-6afd-9168806eeca2@samsung.com>

Andrey Okoshkin <a.okoshkin@samsung.com> writes:

> I'm not sure why only ENOENT error of lstat() is considered as an
> error but passing by other errno values leads to reading of
> uninitialized 'struct stat st' variable.  It means that the
> populated 'diff_filespec' structure may be incorrectly filled.

Entirely correct. There is no fundamental reason to try special casing ENOENT, unless we are clearing the "this is an error" bit when the errno is ENOENT---but this code does not even do so. All errors are errors---we wanted to know the result of lstat() to carry on, and we couldn't figure out the status. We do not want to die immediately (instead we want to show diffs for other paths), so substituting the result with an empty string is the least bad thing we can do at this point in the code.