From: Jeff King <peff@peff.net>
To: Michael Haggerty <mhagger@alum.mit.edu>
Cc: "Junio C Hamano" <gitster@pobox.com>,
git@vger.kernel.org, "Torsten Bögershausen" <tboegi@web.de>,
"Eric Sunshine" <sunshine@sunshineco.com>
Subject: Re: [PATCH v2 00/25] Lockfile correctness and refactoring
Date: Mon, 7 Apr 2014 15:40:44 -0400 [thread overview]
Message-ID: <20140407194043.GE19342@sigill.intra.peff.net> (raw)
In-Reply-To: <1396827247-28465-1-git-send-email-mhagger@alum.mit.edu>
On Mon, Apr 07, 2014 at 01:33:42AM +0200, Michael Haggerty wrote:
> This is a second attempt at renovating the lock file code. Thanks to
> Peff, Junio, Torsten, and Eric for their helpful reviews of v1.
>
> v1 of this patch series [1] did some refactoring and then added a new
> feature to the lock_file API: the ability to activate a new version of
> a locked file while retaining the lock.
>
> But the review of v1 turned up even more correctness issues in the
> existing implementation of lock files. So this v2 dials back the
> scope of the changes (it omits the new feature) but does more work to
> fix problems with the current lock file implementation.
>
> The main theme of this patch series is to better define the state
> diagram for lock_file objects and to fix code that left them in
> incorrect, indeterminate, or unexpected states. There are also a few
> patches that convert several functions to use strbufs instead of
> limiting pathnames to a maximum length.
Looks OK to me, modulo the few comments I sent.
I still think resolve_symref should probably not be "best-effort", but
that can come later on top.
-Peff
prev parent reply other threads:[~2014-04-07 19:40 UTC|newest]
Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-04-06 23:33 [PATCH v2 00/25] Lockfile correctness and refactoring Michael Haggerty
2014-04-06 23:33 ` [PATCH v2 01/25] api-lockfile: expand the documentation Michael Haggerty
2014-04-07 18:46 ` Jeff King
2014-04-06 23:33 ` [PATCH v2 02/25] unable_to_lock_die(): rename function from unable_to_lock_index_die() Michael Haggerty
2014-04-07 18:47 ` Jeff King
2014-04-08 14:04 ` Michael Haggerty
2014-04-06 23:33 ` [PATCH v2 03/25] rollback_lock_file(): do not clear filename redundantly Michael Haggerty
2014-04-06 23:33 ` [PATCH v2 04/25] rollback_lock_file(): set fd to -1 Michael Haggerty
2014-04-06 23:33 ` [PATCH v2 05/25] lockfile: unlock file if lockfile permissions cannot be adjusted Michael Haggerty
2014-04-06 23:33 ` [PATCH v2 06/25] hold_lock_file_for_append(): release lock on errors Michael Haggerty
2014-04-06 23:33 ` [PATCH v2 07/25] lock_file(): always add lock_file object to lock_file_list Michael Haggerty
2014-04-06 23:33 ` [PATCH v2 08/25] struct lock_file: replace on_list field with flags field Michael Haggerty
2014-04-06 23:33 ` [PATCH v2 09/25] lockfile.c: document the various states of lock_file objects Michael Haggerty
2014-04-06 23:33 ` [PATCH v2 10/25] lockfile: define a constant LOCK_SUFFIX_LEN Michael Haggerty
2014-04-06 23:33 ` [PATCH v2 11/25] delete_ref_loose(): don't muck around in the lock_file's filename Michael Haggerty
2014-04-06 23:33 ` [PATCH v2 12/25] prepare_index(): declare return value to be (const char *) Michael Haggerty
2014-04-06 23:33 ` [PATCH v2 13/25] write_packed_entry_fn(): convert cb_data into a (const int *) Michael Haggerty
2014-04-06 23:33 ` [PATCH v2 14/25] lock_file(): exit early if lockfile cannot be opened Michael Haggerty
2014-04-06 23:33 ` [PATCH v2 15/25] remove_lock_file(): call rollback_lock_file() Michael Haggerty
2014-04-06 23:33 ` [PATCH v2 16/25] commit_lock_file(): inline temporary variable Michael Haggerty
2014-04-06 23:33 ` [PATCH v2 17/25] commit_lock_file(): make committing an unlocked lockfile a NOP Michael Haggerty
2014-04-07 19:31 ` Jeff King
2014-04-14 8:21 ` Michael Haggerty
2014-04-06 23:34 ` [PATCH v2 18/25] lockfile: avoid transitory invalid states Michael Haggerty
2014-04-07 6:16 ` Johannes Sixt
2014-04-07 11:13 ` Michael Haggerty
2014-04-07 12:12 ` Johannes Sixt
2014-04-07 13:12 ` Michael Haggerty
2014-04-07 19:38 ` Jeff King
2014-04-06 23:34 ` [PATCH v2 19/25] try_merge_strategy(): remove redundant lock_file allocation Michael Haggerty
2014-04-06 23:34 ` [PATCH v2 20/25] try_merge_strategy(): use a statically-allocated lock_file object Michael Haggerty
2014-04-06 23:34 ` [PATCH v2 21/25] commit_lock_file(): use a strbuf to manage temporary space Michael Haggerty
2014-04-06 23:34 ` [PATCH v2 22/25] Change lock_file::filename into a strbuf Michael Haggerty
2014-04-06 23:34 ` [PATCH v2 23/25] resolve_symlink(): use a strbuf for internal scratch space Michael Haggerty
2014-04-06 23:34 ` [PATCH v2 24/25] resolve_symlink(): take a strbuf parameter Michael Haggerty
2014-04-06 23:34 ` [PATCH v2 25/25] trim_last_path_elm(): replace last_path_elm() Michael Haggerty
2014-04-07 19:40 ` Jeff King [this message]
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=20140407194043.GE19342@sigill.intra.peff.net \
--to=peff@peff.net \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=mhagger@alum.mit.edu \
--cc=sunshine@sunshineco.com \
--cc=tboegi@web.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.