ref_transaction_commit(): use a string_list for detecting duplicates

Detect duplicates by storing the reference names in a string_list and sorting that, instead of sorting the ref_updates directly. * In a moment the string_list will be used for another purpose, too. * This removes the need for the custom comparison function ref_update_compare(). * This means that we can carry out the updates in the order that the user specified them instead of reordering them. This might be handy someday if, we want to permit multiple updates to a single reference as long as they are compatible with each other. Note: we can't use string_list_remove_duplicates() to check for duplicates, because we need to know the name of the reference that appeared multiple times, to be used in the error message. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>

Michael Haggerty committed May 11, 2015 at 17:25 UTC 07f9c881d6acef35b7dc8e5c783e63b1cac71084
1 file changed +11 -14
refs.c
+11 -14
@@ -3720,25 +3720,18 @@ int update_ref(const char *msg, const char *refname,
3720 return 0;
3721 }
3722
3723 -static int ref_update_compare(const void *r1, const void *r2)
3724 -{
3725 - const struct ref_update * const *u1 = r1;
3726 - const struct ref_update * const *u2 = r2;
3727 - return strcmp((*u1)->refname, (*u2)->refname);
3728 -}
3729 -
3730 -static int ref_update_reject_duplicates(struct ref_update **updates, int n,
3723 +static int ref_update_reject_duplicates(struct string_list *refnames,
3724 struct strbuf *err)
3725 {
3733 - int i;
3726 + int i, n = refnames->nr;
3727
3728 assert(err);
3729
3730 for (i = 1; i < n; i++)
3738 - if (!strcmp(updates[i - 1]->refname, updates[i]->refname)) {
3731 + if (!strcmp(refnames->items[i - 1].string, refnames->items[i].string)) {
3732 strbuf_addf(err,
3733 "Multiple updates for ref '%s' not allowed.",
3741 - updates[i]->refname);
3734 + refnames->items[i].string);
3735 return 1;
3736 }
3737 return 0;
@@ -3752,6 +3745,7 @@ int ref_transaction_commit(struct ref_transaction *transaction,
3745 struct ref_update **updates = transaction->updates;
3746 struct string_list refs_to_delete = STRING_LIST_INIT_NODUP;
3747 struct string_list_item *ref_to_delete;
3748 + struct string_list affected_refnames = STRING_LIST_INIT_NODUP;
3749
3750 assert(err);
3751
@@ -3763,9 +3757,11 @@ int ref_transaction_commit(struct ref_transaction *transaction,
3757 return 0;
3758 }
3759
3766 - /* Copy, sort, and reject duplicate refs */
3767 - qsort(updates, n, sizeof(*updates), ref_update_compare);
3768 - if (ref_update_reject_duplicates(updates, n, err)) {
3760 + /* Fail if a refname appears more than once in the transaction: */
3761 + for (i = 0; i < n; i++)
3762 + string_list_append(&affected_refnames, updates[i]->refname);
3763 + string_list_sort(&affected_refnames);
3764 + if (ref_update_reject_duplicates(&affected_refnames, err)) {
3765 ret = TRANSACTION_GENERIC_ERROR;
3766 goto cleanup;
3767 }
@@ -3857,6 +3853,7 @@ cleanup:
3853 if (updates[i]->lock)
3854 unlock_ref(updates[i]->lock);
3855 string_list_clear(&refs_to_delete, 0);
3856 + string_list_clear(&affected_refnames, 0);
3857 return ret;
3858 }
3859