Re: [PATCH v2 2/3] refs/files-backend: stop setting errno from lock_ref_oid_basic

[Junio C Hamano <gitster@pobox.com>]

Han-Wen Nienhuys <hanwen@google.com> writes:

> On Wed, Apr 28, 2021 at 6:20 AM Junio C Hamano <gitster@pobox.com> wrote:
>> > These calls do I/O and therefore clobber errno. They are not inspecting the
>> > incoming errno.
>>
>> Hmph, are you saying that these calls do I/O and always the I/O
>> would fail?  A system call that is successfull don't touch errno;
>> only the calls that resulted in failure do.
>
> I'm saying that callers cannot reliably observe the errno result of
> lock_ref_oid_basic, because it might be clobbered by a failing
> follow-up call.

Sorry, I still do not quite get it.  For example, you cite that a
call to lock_ref_oid_basic() in files_create_symref() is followed by
create_symref_locked() that may clobber errno when the latter fails.

But a failing lock_ref_oid_basic() would yield NULL and causes the
caller to leave, before calling create_symref_locked() and letting
it clobber errno, and the caller of files_create_symref() can
observe, when it returns -1 to signal an error, the errno left by
lock_ref_oid_basic(), no?  I would understand it if no caller of
files_create_symref() cares what is in errno when it receives
negative return to signal a failure, though.

And when lock_ref_oid_basic() did not fail, create_symref_locked()
calls helpers that can fail (e.g. fdopen_lock_file()) and result in
errno getting updated to record how it failed (this is also reported
to the user via "error(... strerror(errno))").

So a caller of files_create_symref() may not be able to tell between
lock_ref_oid_basic() and create_symref_locked() which one caused the
files_create_symref() call to fail, but in either case it should be
able to inspect errno to learn what kind of error we got from the
underlying system, no?