git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Michael Haggerty <mhagger@alum.mit.edu>
To: Junio C Hamano <gitster@pobox.com>,
	David Turner <dturner@twopensource.com>
Cc: "Jeff King" <peff@peff.net>,
	"Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>,
	"Ramsay Jones" <ramsay@ramsayjones.plus.com>,
	git@vger.kernel.org, "Michael Haggerty" <mhagger@alum.mit.edu>
Subject: [PATCH v2 26/33] lock_ref_for_update(): new function
Date: Fri,  6 May 2016 18:14:07 +0200	[thread overview]
Message-ID: <bfba3b42460c75064ce0538cbd6ad820a7ddc537.1462550456.git.mhagger@alum.mit.edu> (raw)
In-Reply-To: <cover.1462550456.git.mhagger@alum.mit.edu>

Extract a new function, lock_ref_for_update(), from
ref_transaction_commit().

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 refs/files-backend.c | 152 ++++++++++++++++++++++++++++-----------------------
 1 file changed, 85 insertions(+), 67 deletions(-)

diff --git a/refs/files-backend.c b/refs/files-backend.c
index f0eef9e..1c20af5 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -3051,6 +3051,88 @@ static int ref_update_reject_duplicates(struct string_list *refnames,
 	return 0;
 }
 
+/*
+ * Acquire all locks, verify old values if provided, check
+ * that new values are valid, and write new values to the
+ * lockfiles, ready to be activated. Only keep one lockfile
+ * open at a time to avoid running out of file descriptors.
+ */
+static int lock_ref_for_update(struct ref_update *update,
+			       struct ref_transaction *transaction,
+			       struct string_list *affected_refnames,
+			       struct strbuf *err)
+{
+	int ret;
+
+	if ((update->flags & REF_HAVE_NEW) &&
+	    is_null_sha1(update->new_sha1))
+		update->flags |= REF_DELETING;
+	update->lock = lock_ref_sha1_basic(
+			update->refname,
+			((update->flags & REF_HAVE_OLD) ?
+			 update->old_sha1 : NULL),
+			affected_refnames, NULL,
+			update->flags,
+			&update->type,
+			err);
+	if (!update->lock) {
+		char *reason;
+
+		ret = (errno == ENOTDIR)
+			? TRANSACTION_NAME_CONFLICT
+			: TRANSACTION_GENERIC_ERROR;
+		reason = strbuf_detach(err, NULL);
+		strbuf_addf(err, "cannot lock ref '%s': %s",
+			    update->refname, reason);
+		free(reason);
+		return ret;
+	}
+	if ((update->flags & REF_HAVE_NEW) &&
+	    !(update->flags & REF_DELETING) &&
+	    !(update->flags & REF_LOG_ONLY)) {
+		int overwriting_symref = ((update->type & REF_ISSYMREF) &&
+					  (update->flags & REF_NODEREF));
+
+		if (!overwriting_symref &&
+		    !hashcmp(update->lock->old_oid.hash, update->new_sha1)) {
+			/*
+			 * The reference already has the desired
+			 * value, so we don't need to write it.
+			 */
+		} else if (write_ref_to_lockfile(update->lock,
+						 update->new_sha1,
+						 err)) {
+			char *write_err = strbuf_detach(err, NULL);
+
+			/*
+			 * The lock was freed upon failure of
+			 * write_ref_to_lockfile():
+			 */
+			update->lock = NULL;
+			strbuf_addf(err,
+				    "cannot update the ref '%s': %s",
+				    update->refname, write_err);
+			free(write_err);
+			return TRANSACTION_GENERIC_ERROR;
+		} else {
+			update->flags |= REF_NEEDS_COMMIT;
+		}
+	}
+	if (!(update->flags & REF_NEEDS_COMMIT)) {
+		/*
+		 * We didn't call write_ref_to_lockfile(), so
+		 * the lockfile is still open. Close it to
+		 * free up the file descriptor:
+		 */
+		if (close_ref(update->lock)) {
+			strbuf_addf(err, "couldn't close '%s.lock'",
+				    update->refname);
+			return TRANSACTION_GENERIC_ERROR;
+		}
+	}
+	return 0;
+}
+
 int ref_transaction_commit(struct ref_transaction *transaction,
 			   struct strbuf *err)
 {
@@ -3088,74 +3170,10 @@ int ref_transaction_commit(struct ref_transaction *transaction,
 	for (i = 0; i < transaction->nr; i++) {
 		struct ref_update *update = updates[i];
 
-		if ((update->flags & REF_HAVE_NEW) &&
-		    is_null_sha1(update->new_sha1))
-			update->flags |= REF_DELETING;
-		update->lock = lock_ref_sha1_basic(
-				update->refname,
-				((update->flags & REF_HAVE_OLD) ?
-				 update->old_sha1 : NULL),
-				&affected_refnames, NULL,
-				update->flags,
-				&update->type,
-				err);
-		if (!update->lock) {
-			char *reason;
-
-			ret = (errno == ENOTDIR)
-				? TRANSACTION_NAME_CONFLICT
-				: TRANSACTION_GENERIC_ERROR;
-			reason = strbuf_detach(err, NULL);
-			strbuf_addf(err, "cannot lock ref '%s': %s",
-				    update->refname, reason);
-			free(reason);
+		ret = lock_ref_for_update(update, transaction,
+					  &affected_refnames, err);
+		if (ret)
 			goto cleanup;
-		}
-		if ((update->flags & REF_HAVE_NEW) &&
-		    !(update->flags & REF_DELETING) &&
-		    !(update->flags & REF_LOG_ONLY)) {
-			int overwriting_symref = ((update->type & REF_ISSYMREF) &&
-						  (update->flags & REF_NODEREF));
-
-			if (!overwriting_symref &&
-			    !hashcmp(update->lock->old_oid.hash, update->new_sha1)) {
-				/*
-				 * The reference already has the desired
-				 * value, so we don't need to write it.
-				 */
-			} else if (write_ref_to_lockfile(update->lock,
-							 update->new_sha1,
-							 err)) {
-				char *write_err = strbuf_detach(err, NULL);
-
-				/*
-				 * The lock was freed upon failure of
-				 * write_ref_to_lockfile():
-				 */
-				update->lock = NULL;
-				strbuf_addf(err,
-					    "cannot update the ref '%s': %s",
-					    update->refname, write_err);
-				free(write_err);
-				ret = TRANSACTION_GENERIC_ERROR;
-				goto cleanup;
-			} else {
-				update->flags |= REF_NEEDS_COMMIT;
-			}
-		}
-		if (!(update->flags & REF_NEEDS_COMMIT)) {
-			/*
-			 * We didn't call write_ref_to_lockfile(), so
-			 * the lockfile is still open. Close it to
-			 * free up the file descriptor:
-			 */
-			if (close_ref(update->lock)) {
-				strbuf_addf(err, "couldn't close '%s.lock'",
-					    update->refname);
-				ret = TRANSACTION_GENERIC_ERROR;
-				goto cleanup;
-			}
-		}
 	}
 
 	/* Perform updates first so live commits remain referenced */
-- 
2.8.1

  parent reply	other threads:[~2016-05-06 16:15 UTC|newest]

Thread overview: 51+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-05-06 16:13 [PATCH v2 00/33] Yet more preparation for reference backends Michael Haggerty
2016-05-06 16:13 ` [PATCH v2 01/33] t1404: demonstrate a bug resolving references Michael Haggerty
2016-05-06 16:13 ` [PATCH v2 02/33] commit_ref(): if there is an empty dir in the way, delete it Michael Haggerty
2016-05-06 16:13 ` [PATCH v2 03/33] read_raw_ref(): don't get confused by an empty directory Michael Haggerty
2016-05-06 16:13 ` [PATCH v2 04/33] safe_create_leading_directories(): improve docstring Michael Haggerty
2016-05-06 16:13 ` [PATCH v2 05/33] remove_dir_recursively(): add docstring Michael Haggerty
2016-05-06 16:13 ` [PATCH v2 06/33] refname_is_safe(): use skip_prefix() Michael Haggerty
2016-05-06 16:13 ` [PATCH v2 07/33] refname_is_safe(): don't allow the empty string Michael Haggerty
2016-05-06 16:13 ` [PATCH v2 08/33] refname_is_safe(): insist that the refname already be normalized Michael Haggerty
2016-05-06 16:13 ` [PATCH v2 09/33] commit_ref_update(): write error message to *err, not stderr Michael Haggerty
2016-05-06 16:13 ` [PATCH v2 10/33] rename_ref(): remove unneeded local variable Michael Haggerty
2016-05-06 16:13 ` [PATCH v2 11/33] ref_transaction_commit(): remove local variable n Michael Haggerty
2016-05-06 16:13 ` [PATCH v2 12/33] read_raw_ref(): rename flags argument to type Michael Haggerty
2016-05-06 16:13 ` [PATCH v2 13/33] read_raw_ref(): clear *type at start of function Michael Haggerty
2016-05-06 16:13 ` [PATCH v2 14/33] read_raw_ref(): rename symref argument to referent Michael Haggerty
2016-05-06 16:13 ` [PATCH v2 15/33] read_raw_ref(): improve docstring Michael Haggerty
2016-05-06 16:13 ` [PATCH v2 16/33] read_raw_ref(): move docstring to header file Michael Haggerty
2016-05-06 16:13 ` [PATCH v2 17/33] lock_ref_sha1_basic(): remove unneeded local variable Michael Haggerty
2016-05-06 16:13 ` [PATCH v2 18/33] refs: make error messages more consistent Michael Haggerty
2016-05-06 16:14 ` [PATCH v2 19/33] ref_transaction_create(): disallow recursive pruning Michael Haggerty
2016-05-06 16:14 ` [PATCH v2 20/33] ref_transaction_commit(): correctly report close_ref() failure Michael Haggerty
2016-05-06 16:14 ` [PATCH v2 21/33] delete_branches(): use resolve_refdup() Michael Haggerty
2016-05-06 16:14 ` [PATCH v2 22/33] refs: allow log-only updates Michael Haggerty
2016-05-06 16:14 ` [PATCH v2 23/33] refs: don't dereference on rename Michael Haggerty
2016-05-06 16:14 ` [PATCH v2 24/33] verify_refname_available(): adjust constness in declaration Michael Haggerty
2016-05-06 16:14 ` [PATCH v2 25/33] add_update(): initialize the whole ref_update Michael Haggerty
2016-05-06 16:14 ` Michael Haggerty [this message]
2016-05-06 16:14 ` [PATCH v2 27/33] unlock_ref(): move definition higher in the file Michael Haggerty
2016-05-06 16:14 ` [PATCH v2 28/33] ref_transaction_update(): check refname_is_safe() at a minimum Michael Haggerty
2016-05-06 16:14 ` [PATCH v2 29/33] refs: resolve symbolic refs first Michael Haggerty
2016-05-12  7:45   ` Jeff King
2016-05-12  8:25     ` Jeff King
2016-05-13 12:33       ` Michael Haggerty
2016-05-13 12:35         ` [PATCH v3 " Michael Haggerty
2016-05-13 12:58           ` Jeff King
2016-05-13 12:51         ` [PATCH v2 " Jeff King
2016-05-14  9:02         ` Torsten Bögershausen
2016-05-06 16:14 ` [PATCH v2 30/33] lock_ref_for_update(): don't re-read non-symbolic references Michael Haggerty
2016-05-06 16:14 ` [PATCH v2 31/33] lock_ref_for_update(): don't resolve symrefs Michael Haggerty
2016-05-06 16:14 ` [PATCH v2 32/33] commit_ref_update(): remove the flags parameter Michael Haggerty
2016-05-06 16:14 ` [PATCH v2 33/33] lock_ref_sha1_basic(): only handle REF_NODEREF mode Michael Haggerty
2016-05-09 20:12 ` [PATCH v2 00/33] Yet more preparation for reference backends David Turner
2016-05-09 21:05   ` Junio C Hamano
2016-05-09 21:50     ` Michael Haggerty
2016-05-09 22:04       ` Junio C Hamano
2016-05-12  7:55         ` Jeff King
2016-05-12 16:10           ` Junio C Hamano
2016-05-10 21:32 ` Junio C Hamano
2016-06-10 12:50 ` Michael Haggerty
2016-06-10 15:43   ` Junio C Hamano
2016-06-13  9:55     ` [ADDENDUM v4] " Michael Haggerty

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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=bfba3b42460c75064ce0538cbd6ad820a7ddc537.1462550456.git.mhagger@alum.mit.edu \
    --to=mhagger@alum.mit.edu \
    --cc=dturner@twopensource.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=pclouds@gmail.com \
    --cc=peff@peff.net \
    --cc=ramsay@ramsayjones.plus.com \
    /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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).