From mboxrd@z Thu Jan 1 00:00:00 1970 From: David Turner Subject: [PATCH v5 14/27] refs: move duplicate check to common code Date: Thu, 18 Feb 2016 00:17:37 -0500 Message-ID: <1455772670-21142-15-git-send-email-dturner@twopensource.com> References: <1455772670-21142-1-git-send-email-dturner@twopensource.com> Cc: David Turner To: git@vger.kernel.org, mhagger@alum.mit.edu X-From: git-owner@vger.kernel.org Thu Feb 18 06:19:07 2016 Return-path: Envelope-to: gcvg-git-2@plane.gmane.org Received: from vger.kernel.org ([209.132.180.67]) by plane.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1aWGza-0001XI-Ie for gcvg-git-2@plane.gmane.org; Thu, 18 Feb 2016 06:19:06 +0100 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1424956AbcBRFTB (ORCPT ); Thu, 18 Feb 2016 00:19:01 -0500 Received: from mail-qk0-f170.google.com ([209.85.220.170]:34966 "EHLO mail-qk0-f170.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1424477AbcBRFSq (ORCPT ); Thu, 18 Feb 2016 00:18:46 -0500 Received: by mail-qk0-f170.google.com with SMTP id o6so14731953qkc.2 for ; Wed, 17 Feb 2016 21:18:46 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=twopensource-com.20150623.gappssmtp.com; s=20150623; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=YpUSTRBl8oBUOaSOjtbpBjshCgeIMRNebAOzSoeJ2S0=; b=fytYHnCRBUYIRQPSRbxWx5BUGeLRjbtn5Gszwl5rvVWxESflvGQCGcNORnzwtY1/6k Ti4dR68nAggO+N3s2FA5u67Fd9PGtxAl9LkjvOEJCduOur3CaFzF3DJA5BO4NxnSckQN cK5RyzMrl6Q7f5NLQOaqlToR2Wn7CxlWS1yUvRjlHntXZYvFgo3v+d9NLv/N7snnZvvi MCXvBy0qhsg/GlVo6fOMfsr2SDtqUtB1zMR+Q4YJyVMKnUZh++qY41Z5w2BpsbT6Q/Hx a2CjpywTtQdCPMUYgBIc81UKrUC/CM9A+8fQmQK4uT8Od4ywdINbkphGB+bI1pr6PYJT UsEQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=YpUSTRBl8oBUOaSOjtbpBjshCgeIMRNebAOzSoeJ2S0=; b=EyjNLWpSnUPXP33SmY3L017A1f5yXTBrb/O2Za60Jq6+eH7DhVQmvv3S+nJxPtgxS9 gcYq7wFvpn9wNY3aaBQJ8UN0v93erR9/hb7vrR1myvP/8YK7NbVmVUn6wZ0nMFk+SHPX VoVtDL+MNcY6nBiHx3jk82ar+pJlTUjWjFCrW6NC8JCA7Z3avKJidCrfS99ZBeWuxBO3 /LgxCGR9qKnkQ6q4xGS00GxFiyT8VjLQ1BrEYpR6aXhTXHzuwDpfX487UDDi4vwACQZM 00QvAXu0G+n5+Q+SFSagmE0fAqSfenneUvpLEkxwVNT1BgiV7+W9BVMatZObJI36+Gtv /IVw== X-Gm-Message-State: AG10YORIxnT9lgtOm90fSwZVc8OaJB9f6fSvmK/d9pCsGQ0WdHxtRsfXg6wzVUV4o616OA== X-Received: by 10.55.71.66 with SMTP id u63mr6554823qka.67.1455772725908; Wed, 17 Feb 2016 21:18:45 -0800 (PST) Received: from ubuntu.twitter.corp? ([8.25.196.26]) by smtp.gmail.com with ESMTPSA id q22sm1965322qkl.19.2016.02.17.21.18.44 (version=TLSv1/SSLv3 cipher=OTHER); Wed, 17 Feb 2016 21:18:45 -0800 (PST) X-Mailer: git-send-email 2.4.2.767.g62658d5-twtrsrc In-Reply-To: <1455772670-21142-1-git-send-email-dturner@twopensource.com> Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Archived-At: 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 --- 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 8eb04da..c9fa34d 100644 --- a/refs.c +++ b/refs.c @@ -1139,6 +1139,36 @@ int head_ref(each_ref_fn fn, void *cb_data) return head_ref_submodule(NULL, fn, 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; +} + int for_each_ref(each_ref_fn fn, void *cb_data) { return do_for_each_ref(NULL, "", fn, 0, 0, cb_data); @@ -1200,7 +1230,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; } const char *resolve_ref_unsafe(const char *ref, int resolve_flags, @@ -1296,7 +1348,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 9011b7c..da06408 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -3076,24 +3076,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; @@ -3101,26 +3085,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 @@ -3139,7 +3103,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); @@ -3251,7 +3215,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; } @@ -3264,27 +3227,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 @@ -3297,7 +3251,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++) { @@ -3307,7 +3261,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; @@ -3338,7 +3292,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 546183d..efdde82 100644 --- a/refs/refs-internal.h +++ b/refs/refs-internal.h @@ -221,6 +221,7 @@ int do_for_each_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