Re: [PATCH 2/2] files_transaction_prepare(): fix handling of ref lock failure

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 25, 2017, 02:29 UTC
Message-ID: <xmqqwp3khsek.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <6214107e1232a7fe7ca4b7440733ff496f07e537.1508856679.git.mhagger@alum.mit.edu>

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

Show 7 quoted lines

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