Re: [PATCH] files_initial_transaction_commit(): only unlock if locked

> On Thu, Jan 18, 2018 at 02:38:41PM +0100, Mathias Rav wrote:
>
>> Running git clone --single-branch --mirror -b TAGNAME previously
>> triggered the following error message:
>> 
>> 	fatal: multiple updates for ref 'refs/tags/TAGNAME' not allowed.
>> 
>> This error condition is handled in files_initial_transaction_commit().
>> 
>> 42c7f7ff9 ("commit_packed_refs(): remove call to `packed_refs_unlock()`", 2017-06-23)
>> introduced incorrect unlocking in the error path of this function,
>> which changes the error message to
>> 
>> 	fatal: BUG: packed_refs_unlock() called when not locked
>> 
>> Move the call to packed_refs_unlock() above the "cleanup:" label
>> since the unlocking should only be done in the last error path.
>
> Thanks, this solution looks correct to me. It's pretty low-impact since
> the locking is the second-to-last thing in the function, so we don't
> have to re-add the unlock to a bunch of error code paths. But one
> alternative would be to just do:
>
>   if (packed_refs_is_locked(refs))
> 	packed_refs_unlock(refs->packed_ref_store);
>
> in the cleanup section.