Johannes Sixt <j6t@xxxxxxxx> writes:
> 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.
Yeah, that is somewhat unfortunate and dubious, but as long as it
works (i.e. various system library calls the codepaths involved
behave sensibly), ...
> What if a subordinate callee starts doing
>
> if (some_syscall(...) < 0) {
> if (errno == EEXIST) {
> /* never mind, that's OK */
> ...
> }
> }
>
> Would it be required to reset errno to its previous value in this
> failure-is-not-an-error case?
... that would be the logical conclusion, I think.
As a longer term fix for better portability (i.e. system libraries
may not behave exactly the way how the codepaths and POSIX expects
wrt to the errors detected), it may become necessary to change the
function to yield "how the call failed" information in addition to
"here is the block of bytes I read for the object", without relying
on particular errno. But as a shorter term solution, ...
> The problem in my Windows build is that one of these subordinate
> syscalls is vsnprintf() (as part of a strbuf_add variant, I presume),
> and it fails with ERANGE when the buffer is too short. Do I have to
> modify the vsnprintf emulation to restore errno?
... I think this is a reasonable thing to do regardless.
Thanks.
