public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed

From: Junio C Hamano <gitster@pobox•com>
To: Jeff King <peff@peff•net>
Cc: Mathias Rav <m@git•strova.dk>,
	git@vger•kernel.org, Michael Haggerty <mhagger@alum•mit.edu>
Subject: Re: [PATCH] files_initial_transaction_commit(): only unlock if locked
Date: Fri, 19 Jan 2018 14:14:07 -0800	[thread overview]
Message-ID: <xmqqwp0do5sg.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <20180118141914.GA32718@sigill.intra.peff.net> (Jeff King's message of "Thu, 18 Jan 2018 09:19:14 -0500")

Jeff King <peff@peff•net> writes:

> 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.

Yeah, that may be a more future-proof alternative, and just as you
said the patch as posted would be sufficient, too.

Thanks.

next prev parent reply	other threads:[~2018-01-19 22:14 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-01-18 13:38 [PATCH] files_initial_transaction_commit(): only unlock if locked Mathias Rav
2018-01-18 14:19 ` Jeff King
2018-01-19 22:14   ` Junio C Hamano [this message]
2018-01-22  9:25     ` Michael Haggerty
2018-01-22 10:03       ` Mathias Rav

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqwp0do5sg.fsf@gitster.mtv.corp.google.com \
    --to=gitster@pobox$(echo .)com \
    --cc=git@vger$(echo .)kernel.org \
    --cc=m@git$(echo .)strova.dk \
    --cc=mhagger@alum$(echo .)mit.edu \
    --cc=peff@peff$(echo .)net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox