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

[PATCH v3 11/20] refs: move duplicate check to common code

From
David Turner <dturner@twopensource.com>
Date
Jan 14, 2016, 16:26 UTC
Message-ID
<1452788777-24954-12-git-send-email-dturner@twopensource.com>
In-Reply-To
<1452788777-24954-1-git-send-email-dturner@twopensource.com>

The check for duplicate refnames in a transaction is needed for all backends, so move it to the common code.

ref_transaction_commit_fn gains a new argument, the sorted string_list of affected refnames.

Signed-off-by: David Turner <dturner@twopensource.com>
---
 refs.c               | 69 ++++++++++++++++++++++++++++++++++++++++++++++++++--
 refs/files-backend.c | 57 ++++---------------------------------------
 refs/refs-internal.h |  1 +
 3 files changed, 73 insertions(+), 54 deletions(-)
diff --git a/refs.c b/refs.c
index deeb7cf..d9cdacf 100644
--- a/refs.c
+++ b/refs.c
@@ -1105,6 +1105,36 @@ int rename_ref_available(const char *oldname, const char *newname)
 	return ret;
 }
 
+/*
+ * Return 1 if there are any duplicate refnames in the updates in
+ * `transaction`, and fill in err with an appropriate error message.
+ * Fill in `refnames` with the refnames from the transaction.
+ */
+static int get_affected_refnames(struct ref_transaction *transaction,
+				 struct string_list *refnames,
+				 struct strbuf *err)
+{
+	int i, n = transaction->nr;
+	struct ref_update **updates;
+
+	assert(err);
+
+	updates = transaction->updates;
+	/* Fail if a refname appears more than once in the transaction: */
+	for (i = 0; i < n; i++)
+		string_list_append(refnames, updates[i]->refname);
+	string_list_sort(refnames);
+
+	for (i = 1; i < n; i++)
+		if (!strcmp(refnames->items[i - 1].string, refnames->items[i].string)) {
+			strbuf_addf(err,
+				    "Multiple updates for ref '%s' not allowed.",
+				    refnames->items[i].string);
+			return 1;
+		}
+	return 0;
+}
+
 /* backend functions */
 int refs_init_db(struct strbuf *err, int shared)
 {
@@ -1114,7 +1144,29 @@ int refs_init_db(struct strbuf *err, int shared)
 int ref_transaction_commit(struct ref_transaction *transaction,
 			   struct strbuf *err)
 {
-	return the_refs_backend->transaction_commit(transaction, err);
+	int ret = -1;
+	struct string_list affected_refnames = STRING_LIST_INIT_NODUP;
+
+	assert(err);
+
+	if (transaction->state != REF_TRANSACTION_OPEN)
+		die("BUG: commit called for transaction that is not open");
+
+	if (!transaction->nr) {
+		transaction->state = REF_TRANSACTION_CLOSED;
+		return 0;
+	}
+
+	if (get_affected_refnames(transaction, &affected_refnames, err)) {
+		ret = TRANSACTION_GENERIC_ERROR;
+		goto done;
+	}
+
+	ret = the_refs_backend->transaction_commit(transaction,
+						   &affected_refnames, err);
+done:
+	string_list_clear(&affected_refnames, 0);
+	return ret;
 }
 
 int delete_refs(struct string_list *refnames)
@@ -1266,5 +1318,18 @@ int reflog_expire(const char *refname, const unsigned char *sha1,
 int initial_ref_transaction_commit(struct ref_transaction *transaction,
 				   struct strbuf *err)
 {
-	return the_refs_backend->initial_transaction_commit(transaction, err);
+	struct string_list affected_refnames = STRING_LIST_INIT_NODUP;
+	int ret;
+
+	if (get_affected_refnames(transaction,
+				  &affected_refnames, err)) {
+		ret = TRANSACTION_GENERIC_ERROR;
+		goto done;
+	}
+	ret = the_refs_backend->initial_transaction_commit(transaction,
+							   &affected_refnames,
+							   err);
+done:
+	string_list_clear(&affected_refnames, 0);
+	return ret;
 }
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 9ae06a6..6dc9257 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -3136,24 +3136,8 @@ static int files_for_each_reflog(each_ref_fn fn, void *cb_data)
 	return retval;
 }
 
-static int ref_update_reject_duplicates(struct string_list *refnames,
-					struct strbuf *err)
-{
-	int i, n = refnames->nr;
-
-	assert(err);
-
-	for (i = 1; i < n; i++)
-		if (!strcmp(refnames->items[i - 1].string, refnames->items[i].string)) {
-			strbuf_addf(err,
-				    "Multiple updates for ref '%s' not allowed.",
-				    refnames->items[i].string);
-			return 1;
-		}
-	return 0;
-}
-
 static int files_transaction_commit(struct ref_transaction *transaction,
+				    struct string_list *affected_refnames,
 				    struct strbuf *err)
 {
 	int ret = 0, i;
@@ -3161,26 +3145,6 @@ static int files_transaction_commit(struct ref_transaction *transaction,
 	struct ref_update **updates = transaction->updates;
 	struct string_list refs_to_delete = STRING_LIST_INIT_NODUP;
 	struct string_list_item *ref_to_delete;
-	struct string_list affected_refnames = STRING_LIST_INIT_NODUP;
-
-	assert(err);
-
-	if (transaction->state != REF_TRANSACTION_OPEN)
-		die("BUG: commit called for transaction that is not open");
-
-	if (!n) {
-		transaction->state = REF_TRANSACTION_CLOSED;
-		return 0;
-	}
-
-	/* Fail if a refname appears more than once in the transaction: */
-	for (i = 0; i < n; i++)
-		string_list_append(&affected_refnames, updates[i]->refname);
-	string_list_sort(&affected_refnames);
-	if (ref_update_reject_duplicates(&affected_refnames, err)) {
-		ret = TRANSACTION_GENERIC_ERROR;
-		goto cleanup;
-	}
 
 	/*
 	 * Acquire all locks, verify old values if provided, check
@@ -3199,7 +3163,7 @@ static int files_transaction_commit(struct ref_transaction *transaction,
 				update->refname,
 				((update->flags & REF_HAVE_OLD) ?
 				 update->old_sha1 : NULL),
-				&affected_refnames, NULL,
+				affected_refnames, NULL,
 				update->flags,
 				&update->type,
 				err);
@@ -3311,7 +3275,6 @@ cleanup:
 		if (updates[i]->backend_data)
 			unlock_ref(updates[i]->backend_data);
 	string_list_clear(&refs_to_delete, 0);
-	string_list_clear(&affected_refnames, 0);
 	return ret;
 }
 
@@ -3324,27 +3287,18 @@ static int ref_present(const char *refname,
 }
 
 static int files_initial_transaction_commit(struct ref_transaction *transaction,
+					    struct string_list *affected_refnames,
 					    struct strbuf *err)
 {
 	int ret = 0, i;
 	int n = transaction->nr;
 	struct ref_update **updates = transaction->updates;
-	struct string_list affected_refnames = STRING_LIST_INIT_NODUP;
 
 	assert(err);
 
 	if (transaction->state != REF_TRANSACTION_OPEN)
 		die("BUG: commit called for transaction that is not open");
 
-	/* Fail if a refname appears more than once in the transaction: */
-	for (i = 0; i < n; i++)
-		string_list_append(&affected_refnames, updates[i]->refname);
-	string_list_sort(&affected_refnames);
-	if (ref_update_reject_duplicates(&affected_refnames, err)) {
-		ret = TRANSACTION_GENERIC_ERROR;
-		goto cleanup;
-	}
-
 	/*
 	 * It's really undefined to call this function in an active
 	 * repository or when there are existing references: we are
@@ -3357,7 +3311,7 @@ static int files_initial_transaction_commit(struct ref_transaction *transaction,
 	 * so here we really only check that none of the references
 	 * that we are creating already exists.
 	 */
-	if (for_each_rawref(ref_present, &affected_refnames))
+	if (for_each_rawref(ref_present, affected_refnames))
 		die("BUG: initial ref transaction called with existing refs");
 
 	for (i = 0; i < n; i++) {
@@ -3367,7 +3321,7 @@ static int files_initial_transaction_commit(struct ref_transaction *transaction,
 		    !is_null_sha1(update->old_sha1))
 			die("BUG: initial ref transaction with old_sha1 set");
 		if (verify_refname_available(update->refname,
-					     &affected_refnames, NULL,
+					     affected_refnames, NULL,
 					     err)) {
 			ret = TRANSACTION_NAME_CONFLICT;
 			goto cleanup;
@@ -3398,7 +3352,6 @@ static int files_initial_transaction_commit(struct ref_transaction *transaction,
 
 cleanup:
 	transaction->state = REF_TRANSACTION_CLOSED;
-	string_list_clear(&affected_refnames, 0);
 	return ret;
 }
 
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index 7390247..0dbb13d 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -217,6 +217,7 @@ int rename_ref_available(const char *oldname, const char *newname);
 /* refs backends */
 typedef int ref_init_db_fn(struct strbuf *err, int shared);
 typedef int ref_transaction_commit_fn(struct ref_transaction *transaction,
+				      struct string_list *affected_refnames,
 				      struct strbuf *err);
 
 /* reflog functions */

-- 
2.4.2.749.g730654d-twtrsrc
Previous: David TurnerNext: David Turner
Message 12 of 49 in “refs backend rebase on pu”
  1. 00/20 refs backend rebase on puDavid Turner, Jan 14, 2016
  2. 01/20 refs: add a backend method structure with transaction functionsDavid Turner, Jan 14, 2016
  3. 02/20 refs: add methods for misc ref operationsDavid Turner, Jan 14, 2016
  4. 03/20 refs: add methods for the ref iteratorsDavid Turner, Jan 14, 2016
  5. 04/20 refs: add do_for_each_per_worktree_refDavid Turner, Jan 14, 2016
  6. 05/20 refs: add methods for reflogDavid Turner, Jan 14, 2016
  7. 06/20 refs: add method for initial ref transaction commitDavid Turner, Jan 14, 2016
  8. 07/20 refs: add method for delete_refsDavid Turner, Jan 14, 2016
  9. 08/20 refs: add methods to init refs dbDavid Turner, Jan 14, 2016
  10. 09/20 refs: add method to rename refsDavid Turner, Jan 14, 2016
  11. 10/20 refs: make lock genericDavid Turner, Jan 14, 2016
  12. 11/20 refs: move duplicate check to common codeDavid Turner, Jan 14, 2016
  13. 12/20 refs: allow log-only updatesDavid Turner, Jan 14, 2016
  14. 13/20 refs: resolve symbolic refs firstDavid Turner, Jan 14, 2016
  15. Jeff KingFeb 4, 2016
  16. David TurnerFeb 4, 2016
  17. 14/20 refs: always handle non-normal refs in files backendDavid Turner, Jan 14, 2016
  18. 15/20 init: allow alternate backends to be set for new reposDavid Turner, Jan 14, 2016
  19. SZEDER GáborJan 15, 2016
  20. Thomas GummererJan 15, 2016
  21. David TurnerJan 19, 2016
  22. Duy NguyenFeb 4, 2016
  23. David TurnerFeb 4, 2016
  24. 16/20 refs: check submodules ref storage configDavid Turner, Jan 14, 2016
  25. 17/20 refs: allow ref backend to be set for cloneDavid Turner, Jan 14, 2016
  26. SZEDER GáborJan 15, 2016
  27. David TurnerJan 19, 2016
  28. SZEDER GáborJan 21, 2016
  29. 18/20 svn: learn ref-storage argumentDavid Turner, Jan 14, 2016
  30. SZEDER GáborJan 15, 2016
  31. 19/20 refs: add LMDB refs backendDavid Turner, Jan 14, 2016
  32. Thomas GummererJan 15, 2016
  33. David TurnerJan 19, 2016
  34. Duy NguyenFeb 4, 2016
  35. David TurnerFeb 4, 2016
  36. 20/20 refs: tests for lmdb backendDavid Turner, Jan 14, 2016
  37. David TurnerFeb 2, 2016
  38. Junio C HamanoFeb 2, 2016
  39. Junio C HamanoFeb 4, 2016
  40. David TurnerFeb 4, 2016
  41. Ramsay JonesFeb 4, 2016
  42. Jeff KingFeb 4, 2016
  43. David TurnerFeb 4, 2016
  44. Duy NguyenFeb 4, 2016
  45. David TurnerFeb 4, 2016
  46. Duy NguyenFeb 4, 2016
  47. David TurnerFeb 4, 2016
  48. Ramsay JonesFeb 4, 2016
  49. David TurnerFeb 4, 2016

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.