public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox•com>
To: Michael Haggerty <mhagger@alum•mit.edu>
Cc: Jeff King <peff@peff•net>, git@vger•kernel.org
Subject: Re: [PATCH 2/2] files_transaction_prepare(): fix handling of ref lock failure
Date: Wed, 25 Oct 2017 11:29:07 +0900	[thread overview]
Message-ID: <xmqqwp3khsek.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <6214107e1232a7fe7ca4b7440733ff496f07e537.1508856679.git.mhagger@alum.mit.edu> (Michael Haggerty's message of "Tue, 24 Oct 2017 17:16:25 +0200")

Michael Haggerty <mhagger@alum•mit.edu> writes:

> ... But dc39e09942 added another blurb of code between
> the loop and the cleanup. That blurb sometimes resets `ret` to zero,
> making the cleanup code think that the locking was successful.
> ...
> The fix is simple: instead of just breaking out of the loop, jump
> directly to the cleanup code. This fixes some tests in t1404 that were
> added in the previous commit.

OK.  Now because we do not break and start packed_transaction but
instead jump over that if statement, we'll leave packed_transation
instance that we got from transaction_begin() that we called
add_update() on, but haven't called transaction_prepare() on
behind.  That instance is pointed by backend_data pointer which is
part of transaction, so presumably transaction_cleanup() called on
it in the section labelled "cleanup:" should take care of it?

Thanks for catching the issue and fixing it quickly.  Will queue.

  parent reply	other threads:[~2017-10-25  2:29 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-10-24 15:16 [PATCH 0/2] Fix an error-handling path when locking refs Michael Haggerty
2017-10-24 15:16 ` [PATCH 1/2] t1404: add a bunch of tests of D/F conflicts Michael Haggerty
2017-10-24 16:19   ` Eric Sunshine
2017-10-24 19:46     ` Jeff King
2017-10-25  8:03       ` Michael Haggerty
2017-10-25  8:23         ` Michael Haggerty
2017-10-26  6:32         ` Jeff King
2017-10-28 16:42           ` brian m. carlson
2017-10-29  0:05             ` Junio C Hamano
2017-10-29 16:12               ` brian m. carlson
2017-10-24 15:16 ` [PATCH 2/2] files_transaction_prepare(): fix handling of ref lock failure Michael Haggerty
2017-10-24 19:49   ` Jeff King
2017-10-25  2:29   ` Junio C Hamano [this message]
2017-10-24 19:45 ` [PATCH 0/2] Fix an error-handling path when locking refs Jeff King

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=xmqqwp3khsek.fsf@gitster.mtv.corp.google.com \
    --to=gitster@pobox$(echo .)com \
    --cc=git@vger$(echo .)kernel.org \
    --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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox