From: "René Scharfe" <l.s.r@web.de>
To: Michael Haggerty <mhagger@alum.mit.edu>,
Junio C Hamano <gitster@pobox.com>
Cc: "Johannes Sixt" <j6t@kdbg.org>,
"Torsten Bögershausen" <tboegi@web.de>,
"Jeff King" <peff@peff.net>,
"Ronnie Sahlberg" <sahlberg@google.com>,
"Jonathan Nieder" <jrnieder@gmail.com>,
git@vger.kernel.org
Subject: Re: [PATCH v7 09/38] lock_file(): always initialize and register lock_file object
Date: Wed, 01 Oct 2014 13:27:49 +0200 [thread overview]
Message-ID: <542BE535.3040500@web.de> (raw)
In-Reply-To: <1412159322-2622-10-git-send-email-mhagger@alum.mit.edu>
Am 01.10.2014 um 12:28 schrieb Michael Haggerty:
> The purpose of this patch is to make the state diagram for lock_file
> objects simpler and deterministic.
>
> If locking fails, lock_file() sometimes leaves the lock_file object
> partly initialized, but sometimes not. It sometimes registers the
> object in lock_file_list, but sometimes not. This makes the state
> diagram for lock_file objects effectively indeterministic and hard to
> reason about. A future patch will also change the filename field into
> a strbuf, which needs more involved initialization, so it will become
> even more important that the state of a lock_file object is
> well-defined after a failed attempt to lock.
>
> The ambiguity doesn't currently have any ill effects, because
> lock_file objects cannot be removed from the lock_file_list anyway.
> But to make it easier to document and reason about the code, make this
> behavior inconsistent: *always* initialize the lock_file object and
s/incon/con/, certainly?
> *always* register it in lock_file_list the first time it is used,
> regardless of whether an error occurs.
>
> While we're at it, make sure that all of the lock_file fields are
> initialized to values appropriate for an unlocked object; the caller
> is only responsible for making sure that on_list is set to zero before
> the first time it is used.
next prev parent reply other threads:[~2014-10-01 11:29 UTC|newest]
Thread overview: 43+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-10-01 10:28 [PATCH v7 00/38] Lockfile correctness and refactoring Michael Haggerty
2014-10-01 10:28 ` [PATCH v7 01/38] unable_to_lock_die(): rename function from unable_to_lock_index_die() Michael Haggerty
2014-10-01 10:28 ` [PATCH v7 02/38] api-lockfile: revise and expand the documentation Michael Haggerty
2014-10-01 10:28 ` [PATCH v7 03/38] close_lock_file(): exit (successfully) if file is already closed Michael Haggerty
2014-10-01 10:28 ` [PATCH v7 04/38] rollback_lock_file(): do not clear filename redundantly Michael Haggerty
2014-10-01 10:28 ` [PATCH v7 05/38] rollback_lock_file(): exit early if lock is not active Michael Haggerty
2014-10-01 10:28 ` [PATCH v7 06/38] rollback_lock_file(): set fd to -1 Michael Haggerty
2014-10-01 10:28 ` [PATCH v7 07/38] lockfile: unlock file if lockfile permissions cannot be adjusted Michael Haggerty
2014-10-01 10:28 ` [PATCH v7 08/38] hold_lock_file_for_append(): release lock on errors Michael Haggerty
2014-10-01 10:28 ` [PATCH v7 09/38] lock_file(): always initialize and register lock_file object Michael Haggerty
2014-10-01 11:27 ` René Scharfe [this message]
2014-10-01 11:38 ` Michael Haggerty
2014-10-01 20:36 ` Junio C Hamano
2014-10-01 10:28 ` [PATCH v7 10/38] lockfile.c: document the various states of lock_file objects Michael Haggerty
2014-10-01 10:28 ` [PATCH v7 11/38] cache.h: define constants LOCK_SUFFIX and LOCK_SUFFIX_LEN Michael Haggerty
2014-10-01 10:28 ` [PATCH v7 12/38] delete_ref_loose(): don't muck around in the lock_file's filename Michael Haggerty
2014-10-01 10:28 ` [PATCH v7 13/38] prepare_index(): declare return value to be (const char *) Michael Haggerty
2014-10-01 10:28 ` [PATCH v7 14/38] lock_file(): exit early if lockfile cannot be opened Michael Haggerty
2014-10-01 10:28 ` [PATCH v7 15/38] remove_lock_file(): call rollback_lock_file() Michael Haggerty
2014-10-01 10:28 ` [PATCH v7 16/38] commit_lock_file(): inline temporary variable Michael Haggerty
2014-10-01 10:28 ` [PATCH v7 17/38] commit_lock_file(): die() if called for unlocked lockfile object Michael Haggerty
2014-10-01 10:28 ` [PATCH v7 18/38] close_lock_file(): if close fails, roll back Michael Haggerty
2014-10-01 10:28 ` [PATCH v7 19/38] commit_lock_file(): rollback lock file on failure to rename Michael Haggerty
2014-10-01 10:28 ` [PATCH v7 20/38] api-lockfile: document edge cases Michael Haggerty
2014-10-01 10:28 ` [PATCH v7 21/38] dump_marks(): remove a redundant call to rollback_lock_file() Michael Haggerty
2014-10-01 10:28 ` [PATCH v7 22/38] git_config_set_multivar_in_file(): avoid " Michael Haggerty
2014-10-01 10:28 ` [PATCH v7 23/38] lockfile: avoid transitory invalid states Michael Haggerty
2014-10-01 10:28 ` [PATCH v7 24/38] struct lock_file: declare some fields volatile Michael Haggerty
2014-10-01 10:28 ` [PATCH v7 25/38] try_merge_strategy(): remove redundant lock_file allocation Michael Haggerty
2014-10-01 10:28 ` [PATCH v7 26/38] try_merge_strategy(): use a statically-allocated lock_file object Michael Haggerty
2014-10-01 10:28 ` [PATCH v7 27/38] commit_lock_file(): use a strbuf to manage temporary space Michael Haggerty
2014-10-01 10:28 ` [PATCH v7 28/38] Change lock_file::filename into a strbuf Michael Haggerty
2014-10-01 10:28 ` [PATCH v7 29/38] resolve_symlink(): use a strbuf for internal scratch space Michael Haggerty
2014-10-01 10:28 ` [PATCH v7 30/38] resolve_symlink(): take a strbuf parameter Michael Haggerty
2014-10-01 10:28 ` [PATCH v7 31/38] trim_last_path_component(): replace last_path_elm() Michael Haggerty
2014-10-01 10:28 ` [PATCH v7 32/38] Extract a function commit_lock_file_to() Michael Haggerty
2014-10-01 10:28 ` [PATCH v7 33/38] Rename LOCK_NODEREF to LOCK_NO_DEREF Michael Haggerty
2014-10-01 10:28 ` [PATCH v7 34/38] lockfile.c: rename static functions Michael Haggerty
2014-10-01 10:28 ` [PATCH v7 35/38] get_locked_file_path(): new function Michael Haggerty
2014-10-01 10:28 ` [PATCH v7 36/38] hold_lock_file_for_append(): restore errno before returning Michael Haggerty
2014-10-01 10:28 ` [PATCH v7 37/38] Move read_index() definition to read-cache.c Michael Haggerty
2014-10-01 10:28 ` [PATCH v7 38/38] lockfile.h: extract new header file for the functions in lockfile.c Michael Haggerty
2014-10-01 21:05 ` [PATCH v7 00/38] Lockfile correctness and refactoring Junio C Hamano
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=542BE535.3040500@web.de \
--to=l.s.r@web.de \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=j6t@kdbg.org \
--cc=jrnieder@gmail.com \
--cc=mhagger@alum.mit.edu \
--cc=peff@peff.net \
--cc=sahlberg@google.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.