Re: ERANGE strikes again on my Windows build; RFH
- From
Jonathan Nieder <jrnieder@gmail.com>
- Date
- Dec 30, 2019, 18:06 UTC
- Message-ID
- <20191230180653.GA57251@google.com>
- In-Reply-To
- <6eb02a73-9dcb-f1fc-f015-80e71e9910d6@kdbg.org>
Hi,
Johannes Sixt wrote:
Show 13 quoted lines
> 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