git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v2 10/12] ref_transaction_verify(): new function to check a reference's value

From
Michael Haggerty <mhagger@alum.mit.edu>
Date
Feb 12, 2015, 11:12 UTC
Message-ID
<1423739543-1025-11-git-send-email-mhagger@alum.mit.edu>
In-Reply-To
<1423739543-1025-1-git-send-email-mhagger@alum.mit.edu>

If NULL is passed to ref_transaction_update()'s new_sha1 parameter, then just verify old_sha1 (under lock) without trying to change the new value of the reference.

Use this functionality to add a new function ref_transaction_verify(), which checks the current value of the reference under lock but doesn't change it.

Use ref_transaction_verify() in the implementation of "git update-ref --stdin"'s "verify" command to avoid the awkward need to "update" the reference to its existing value.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 builtin/update-ref.c |  7 ++-----
 refs.c               | 47 +++++++++++++++++++++++++++++++++++++++--------
 refs.h               | 34 ++++++++++++++++++++++++++--------
 3 files changed, 67 insertions(+), 21 deletions(-)
diff --git a/builtin/update-ref.c b/builtin/update-ref.c
index e9632d5..a75a098 100644
--- a/builtin/update-ref.c
+++ b/builtin/update-ref.c
@@ -282,7 +282,6 @@ static const char *parse_cmd_verify(struct ref_transaction *transaction,
 {
 	struct strbuf err = STRBUF_INIT;
 	char *refname;
-	unsigned char new_sha1[20];
 	unsigned char old_sha1[20];
 
 	refname = parse_refname(input, &next);
@@ -293,13 +292,11 @@ static const char *parse_cmd_verify(struct ref_transaction *transaction,
 			    PARSE_SHA1_OLD))
 		hashclr(old_sha1);
 
-	hashcpy(new_sha1, old_sha1);
-
 	if (*next != line_termination)
 		die("verify %s: extra input: %s", refname, next);
 
-	if (ref_transaction_update(transaction, refname, new_sha1, old_sha1,
-				   update_flags, msg, &err))
+	if (ref_transaction_verify(transaction, refname, old_sha1,
+				   update_flags, &err))
 		die("%s", err.buf);
 
 	update_flags = 0;
diff --git a/refs.c b/refs.c
index 9dd7932..489eec9 100644
--- a/refs.c
+++ b/refs.c
@@ -47,10 +47,16 @@ static unsigned char refname_disposition[256] = {
 #define REF_ISPRUNING	0x04
 
 /*
+ * Used as a flag in ref_update::flags when the reference should be
+ * updated to new_sha1.
+ */
+#define REF_HAVE_NEW	0x08
+
+/*
  * Used as a flag in ref_update::flags when old_sha1 should be
  * checked.
  */
-#define REF_HAVE_OLD	0x08
+#define REF_HAVE_OLD	0x10
 
 /*
  * Try to read one refname component from the front of refname.
@@ -3577,10 +3583,17 @@ int for_each_reflog(each_ref_fn fn, void *cb_data)
  * not exist before update.
  */
 struct ref_update {
+	/*
+	 * If (flags & REF_HAVE_NEW), set the reference to this value:
+	 */
 	unsigned char new_sha1[20];
+	/*
+	 * If (flags & REF_HAVE_OLD), check that the reference
+	 * previously had this value:
+	 */
 	unsigned char old_sha1[20];
 	/*
-	 * One or more of REF_HAVE_OLD, REF_NODEREF,
+	 * One or more of REF_HAVE_NEW, REF_HAVE_OLD, REF_NODEREF,
 	 * REF_DELETING, and REF_IS_PRUNING:
 	 */
 	int flags;
@@ -3665,7 +3678,7 @@ int ref_transaction_update(struct ref_transaction *transaction,
 	if (transaction->state != REF_TRANSACTION_OPEN)
 		die("BUG: update called for transaction that is not open");
 
-	if (!is_null_sha1(new_sha1) &&
+	if (new_sha1 && !is_null_sha1(new_sha1) &&
 	    check_refname_format(refname, REFNAME_ALLOW_ONELEVEL)) {
 		strbuf_addf(err, "refusing to update ref with bad name %s",
 			    refname);
@@ -3673,7 +3686,10 @@ int ref_transaction_update(struct ref_transaction *transaction,
 	}
 
 	update = add_update(transaction, refname);
-	hashcpy(update->new_sha1, new_sha1);
+	if (new_sha1) {
+		hashcpy(update->new_sha1, new_sha1);
+		flags |= REF_HAVE_NEW;
+	}
 	if (old_sha1) {
 		hashcpy(update->old_sha1, old_sha1);
 		flags |= REF_HAVE_OLD;
@@ -3709,6 +3725,19 @@ int ref_transaction_delete(struct ref_transaction *transaction,
 				      flags, msg, err);
 }
 
+int ref_transaction_verify(struct ref_transaction *transaction,
+			   const char *refname,
+			   const unsigned char *old_sha1,
+			   int flags,
+			   struct strbuf *err)
+{
+	if (!old_sha1)
+		die("BUG: verify called with old_sha1 set to NULL");
+	return ref_transaction_update(transaction, refname,
+				      NULL, old_sha1,
+				      flags, NULL, err);
+}
+
 int update_ref(const char *action, const char *refname,
 	       const unsigned char *sha1, const unsigned char *oldval,
 	       int flags, enum action_on_err onerr)
@@ -3797,7 +3826,7 @@ int ref_transaction_commit(struct ref_transaction *transaction,
 		struct ref_update *update = updates[i];
 		int flags = update->flags;
 
-		if (is_null_sha1(update->new_sha1))
+		if ((flags & REF_HAVE_NEW) && is_null_sha1(update->new_sha1))
 			flags |= REF_DELETING;
 		update->lock = lock_ref_sha1_basic(
 				update->refname,
@@ -3819,8 +3848,9 @@ int ref_transaction_commit(struct ref_transaction *transaction,
 	/* Perform updates first so live commits remain referenced */
 	for (i = 0; i < n; i++) {
 		struct ref_update *update = updates[i];
+		int flags = update->flags;
 
-		if (!is_null_sha1(update->new_sha1)) {
+		if ((flags & REF_HAVE_NEW) && !is_null_sha1(update->new_sha1)) {
 			if (write_ref_sha1(update->lock, update->new_sha1,
 					   update->msg)) {
 				update->lock = NULL; /* freed by write_ref_sha1 */
@@ -3836,14 +3866,15 @@ int ref_transaction_commit(struct ref_transaction *transaction,
 	/* Perform deletes now that updates are safely completed */
 	for (i = 0; i < n; i++) {
 		struct ref_update *update = updates[i];
+		int flags = update->flags;
 
-		if (update->lock) {
+		if ((flags & REF_HAVE_NEW) && is_null_sha1(update->new_sha1)) {
 			if (delete_ref_loose(update->lock, update->type, err)) {
 				ret = TRANSACTION_GENERIC_ERROR;
 				goto cleanup;
 			}
 
-			if (!(update->flags & REF_ISPRUNING))
+			if (!(flags & REF_ISPRUNING))
 				string_list_append(&refs_to_delete,
 						   update->lock->ref_name);
 		}
diff --git a/refs.h b/refs.h
index ee612da..02d6e39 100644
--- a/refs.h
+++ b/refs.h
@@ -263,14 +263,19 @@ struct ref_transaction *ref_transaction_begin(struct strbuf *err);
  */
 
 /*
- * Add a reference update to transaction.  new_sha1 is the value that
- * the reference should have after the update, or null_sha1 if it should
- * be deleted.  If old_sha1 is non-NULL, then it the value
- * that the reference should have had before the update, or null_sha1 if
- * it must not have existed beforehand.
- * Function returns 0 on success and non-zero on failure. A failure to update
- * means that the transaction as a whole has failed and will need to be
- * rolled back.
+ * Add a reference update to transaction. new_sha1 is the value that
+ * the reference should have after the update, or null_sha1 if it
+ * should be deleted. If new_sha1 is NULL, then the reference is not
+ * changed at all. old_sha1 is the value that the reference must have
+ * before the update, or null_sha1 if it must not have existed
+ * beforehand. The old value is checked after the lock is taken to
+ * prevent races. If the old value doesn't agree with old_sha1, the
+ * whole transaction fails. If old_sha1 is NULL, then the previous
+ * value is not checked.
+ *
+ * Return 0 on success and non-zero on failure. Any failure in the
+ * transaction means that the transaction as a whole has failed and
+ * will need to be rolled back.
  */
 int ref_transaction_update(struct ref_transaction *transaction,
 			   const char *refname,
@@ -309,6 +314,19 @@ int ref_transaction_delete(struct ref_transaction *transaction,
 			   struct strbuf *err);
 
 /*
+ * Verify, within a transaction, that refname has the value old_sha1,
+ * or, if old_sha1 is null_sha1, then verify that the reference
+ * doesn't exist. old_sha1 must be non-NULL. Function returns 0 on
+ * success and non-zero on failure. A failure to verify means that the
+ * transaction as a whole has failed and will need to be rolled back.
+ */
+int ref_transaction_verify(struct ref_transaction *transaction,
+			   const char *refname,
+			   const unsigned char *old_sha1,
+			   int flags,
+			   struct strbuf *err);
+
+/*
  * Commit all of the changes that have been queued in transaction, as
  * atomically as possible.
  *

-- 
2.1.4
Previous: Michael HaggertyNext: Stefan Beller
Message 23 of 27 in “Allow reference values to be checked in a transaction”
  1. 00/12 Allow reference values to be checked in a transactionMichael Haggerty, Feb 12, 2015
  2. 01/12 refs: move REF_DELETING to refs.cMichael Haggerty, Feb 12, 2015
  3. 02/12 refs: remove the gap in the REF_* constant valuesMichael Haggerty, Feb 12, 2015
  4. 03/12 struct ref_update: move "have_old" into "flags"Michael Haggerty, Feb 12, 2015
  5. Stefan BellerFeb 12, 2015
  6. Junio C HamanoFeb 12, 2015
  7. Michael HaggertyFeb 17, 2015
  8. Junio C HamanoFeb 17, 2015
  9. 04/12 ref_transaction_update(): remove "have_old" parameterMichael Haggerty, Feb 12, 2015
  10. Junio C HamanoFeb 12, 2015
  11. Michael HaggertyFeb 13, 2015
  12. Junio C HamanoFeb 13, 2015
  13. 05/12 ref_transaction_delete(): remove "have_old" parameterMichael Haggerty, Feb 12, 2015
  14. 06/12 commit: add tests of commit racesMichael Haggerty, Feb 12, 2015
  15. Stefan BellerFeb 12, 2015
  16. Michael HaggertyFeb 17, 2015
  17. Junio C HamanoFeb 12, 2015
  18. Michael HaggertyFeb 17, 2015
  19. 07/12 commit: avoid race when creating orphan commitsMichael Haggerty, Feb 12, 2015
  20. Junio C HamanoFeb 12, 2015
  21. 08/12 ref_transaction_create(): check that new_sha1 is validMichael Haggerty, Feb 12, 2015
  22. 09/12 ref_transaction_delete(): check that old_sha1 is not null_sha1Michael Haggerty, Feb 12, 2015
  23. 10/12 ref_transaction_verify(): new function to check a reference's valueMichael Haggerty, Feb 12, 2015
  24. Stefan BellerFeb 12, 2015
  25. 11/12 update_ref(): improve documentationMichael Haggerty, Feb 12, 2015
  26. 12/12 refs.h: Remove duplication in function docstringsMichael Haggerty, Feb 12, 2015
  27. Junio C HamanoFeb 12, 2015

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.