Subject: Re: ERANGE strikes again on my Windows build; RFH

From: Jonathan Nieder <[email-masked]>
Date: Mon, 30 Dec 2019 10:06:53 -0800

Hi,

Johannes Sixt wrote:

> In sha1-file.c:read_object_file_extended() we have the following pattern:
>
> 	errno = 0;
> 	data = read_object(r, repl, type, size);
> 	if (data)
> 		return data;
>
> 	if (errno && errno != ENOENT)
> 		die_errno(_("failed to read object %s"), oid_to_hex(oid));
>
> That is, it is expected that read_object() does not change the value of
> errno in the non-error case. I find it intriguing that we expect a quite
> large call graph that is behind read_object() to behave this way.

Yes, this seems dubious.

In fact this is only inspecting errno in the returned-NULL case.  If I
look only at the code above and not at the implementation of
read_object, then I would say that the bug is the 'errno &&' part: when
errno is meaningful for a function for a given return value, the usual
convention is

 (1) it *always* sets errno for errors, not conditionally
 (2) it never sets errno to 0

There are some exceptions (like strtoul) but they are few and
unfortunate, not something to be imitated.

Do you have more details about the case where read_object is expected
to produce errno == 0?  I'm wondering whether we forgot to set 'errno
= ENOENT' explicitly somewhere.

Thanks and hope that helps,
Jonathan