Re: [PATCH 2/5] refs.c: return error instead of dying when locking fails during transaction

From: Ronnie Sahlberg <sahlberg@google.com>
Date: Jul 28, 2014, 19:01 UTC
Message-ID: <CAL=YDWmCGMKgmc6NOhHuf-1QXFvnF25yT-WgxJBntu+DGAbBmg@mail.gmail.com>
In-Reply-To: <20140725194023.GH12427@google.com>

On Fri, Jul 25, 2014 at 12:40 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:

Show 13 quoted lines

> Ronnie Sahlberg wrote:
>
>> --- a/refs.c
>> +++ b/refs.c
>> @@ -2214,7 +2214,7 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname,
>>                        */
>>                       goto retry;
>>               else
>> -                     unable_to_lock_index_die(ref_file, errno);
>> +                     goto error_return;
>
> Should probably save last_errno so error_return can pass that
> information back.

Done. Thanks.

Show 7 quoted lines

>
> Can the caller recover from this error?  Does it have enough information
> to produce the same helpful message as unable_to_lock_index_die?
>
> If this could be done without regressing behavior (e.g., by passing
> back error information as a message instead of through errno) then I
> think it would make sense.

The callers should all be able to cope with this returning error (and now logging error() since this function is only called from within transaction_commit() and it knows how to deal with these errors.

>
> Jonathan