Am 30.12.19 um 19:06 schrieb Jonathan Nieder:
> 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
You seem to understand that errno isn't set somewhere where it should be
set. But the problem is not absence of setting errno, but abundance of
setting errno; in particular, when functions receive errors from
lower-level functions, but then indicate to the higher levels that
everything's fine; then the higher levels observe errno when they shouldn't.
> (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.
I don't think that forgetting to set ENOENT is the problem.
It happens reproducibly in test 5 of t0410-partial-clone:
++ git -C repo fsck
Checking object directories: 100% (256/256), done.
Checking objects: 100% (1/1), done.
fatal: failed to read object
383670739c2f993999f3c10911ac5cb5c6591523: Result too large
when it should be
Checking object directories: 100% (256/256), done.
Checking objects: 100% (1/1), done.
dangling tag e5f4cb9fd329c512b08fb81a8e6b1f5e27658263
-- Hannes
