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 <dtur...@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 05b1b2a..daf92ce 100644
--- a/refs.c
+++ b/refs.c
@@ -1140,6 +1140,36 @@ int head_ref(each_ref_fn fn, void *cb_data)
 }
 
 /*
+ * 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;
+}
+
+/*
  * The common backend for the for_each_*ref* functions
  */
 static int do_for_each_ref(const char *submodule, const char *base,
@@ -1324,7 +1354,29 @@ int refs_init_db(int shared, struct strbuf *err)
 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 verify_refname_available(const char *refname, struct string_list *extra,
@@ -1405,7 +1457,20 @@ 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;
 }
 
 int delete_refs(struct string_list *refnames)
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 9be3158..413db22 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -3020,24 +3020,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;
@@ -3045,26 +3029,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
@@ -3083,7 +3047,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);
@@ -3195,7 +3159,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;
 }
 
@@ -3208,27 +3171,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
@@ -3241,7 +3195,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++) {
@@ -3251,7 +3205,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;
@@ -3282,7 +3236,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 8d091cb..94d162e 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -218,6 +218,7 @@ int do_for_each_per_worktree_ref(const char *submodule, 
const char *base,
 /* refs backends */
 typedef int ref_init_db_fn(int shared, struct strbuf *err);
 typedef int ref_transaction_commit_fn(struct ref_transaction *transaction,
+                                     struct string_list *affected_refnames,
                                      struct strbuf *err);
 
 /* reflog functions */
-- 
2.4.2.767.g62658d5-twtrsrc

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to