git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: mhagger@alum.mit.edu
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org, Jeff King <peff@peff.net>,
	Drew Northup <drew.northup@maine.edu>,
	Jakub Narebski <jnareb@gmail.com>,
	Heiko Voigt <hvoigt@hvoigt.net>,
	Johan Herland <johan@herland.net>,
	Julian Phillips <julian@quantumfyre.co.uk>,
	Michael Haggerty <mhagger@alum.mit.edu>
Subject: [PATCH v2 49/51] is_refname_available(): query only possibly-conflicting references
Date: Mon, 12 Dec 2011 06:38:56 +0100	[thread overview]
Message-ID: <1323668338-1764-50-git-send-email-mhagger@alum.mit.edu> (raw)
In-Reply-To: <1323668338-1764-1-git-send-email-mhagger@alum.mit.edu>

From: Michael Haggerty <mhagger@alum.mit.edu>

Instead of iterating through all of the references, inquire more
pointedly about the references that could conflict with the new name.
This requires checking for a few individual references, plus iterating
through a small subtree of the rest of the references (and usually the
subtree iteration ends without having to recurse).  A big benefit is
that populating the whole loose reference cache (which can be very
expensive) can usually be avoided.

Also, simplify name_conflict_fn().  Since it now will only be called
for possibly-conflicting references, there is necessarily a conflict
if it is called for *any* reference besides "oldrefname".

Remove function names_conflict(), which is now unused.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 refs.c |   74 ++++++++++++++++++++++++++++++++++++++++-----------------------
 1 files changed, 47 insertions(+), 27 deletions(-)

diff --git a/refs.c b/refs.c
index 3cd8e04..312ca3b 100644
--- a/refs.c
+++ b/refs.c
@@ -591,23 +591,7 @@ static int do_for_each_ref_in_dirs(struct ref_entry *direntry1,
 	}
 }
 
-/*
- * Return true iff refname1 and refname2 conflict with each other.
- * Two reference names conflict if one of them exactly matches the
- * leading components of the other; e.g., "foo/bar" conflicts with
- * both "foo" and with "foo/bar/baz" but not with "foo/bar" or
- * "foo/barbados".
- */
-static int names_conflict(const char *refname1, const char *refname2)
-{
-	for (; *refname1 && *refname1 == *refname2; refname1++, refname2++)
-		;
-	return (*refname1 == '\0' && *refname2 == '/')
-		|| (*refname1 == '/' && *refname2 == '\0');
-}
-
 struct name_conflict_cb {
-	const char *refname;
 	const char *oldrefname;
 	const char *conflicting_refname;
 };
@@ -616,31 +600,67 @@ static int name_conflict_fn(const char *existingrefname, const unsigned char *sh
 			    int flags, void *cb_data)
 {
 	struct name_conflict_cb *data = (struct name_conflict_cb *)cb_data;
-	if (data->oldrefname && !strcmp(data->oldrefname, existingrefname))
+	if (!strcmp(data->oldrefname, existingrefname))
 		return 0;
-	if (names_conflict(data->refname, existingrefname)) {
-		data->conflicting_refname = existingrefname;
-		return 1;
-	}
-	return 0;
+
+	/*
+	 * Since we are only iterating over the subtree that has the
+	 * new refname as prefix, *any* reference found is a conflict.
+	 */
+	data->conflicting_refname = existingrefname;
+	return 1;
 }
 
 /*
  * Return true iff a reference named refname could be created without
- * conflicting with the name of an existing reference.  If oldrefname
- * is non-NULL, ignore potential conflicts with oldrefname (e.g.,
- * because oldrefname is scheduled for deletion in the same
+ * conflicting with the name of an existing reference in direntry.  If
+ * oldrefname is non-NULL, ignore potential conflicts with oldrefname
+ * (e.g., because oldrefname is scheduled for deletion in the same
  * operation).
+ *
+ * Two reference names conflict if one of them exactly matches the
+ * leading components of the other; e.g., "foo/bar" conflicts with
+ * both "foo" and with "foo/bar/baz" but not with "foo/bar" or
+ * "foo/barbados".
  */
 static int is_refname_available(const char *refname, const char *oldrefname,
 				struct ref_entry *direntry)
 {
+	int prefixlen = strlen(refname);
+	char *prefix;
+	char *slash;
 	struct name_conflict_cb data;
-	data.refname = refname;
+
+	assert(direntry->flag & REF_DIR);
+
+	if (!oldrefname)
+		oldrefname = ""; /* invalid; cannot match any existing refname */
+
+	/* Check whether a prefix of refname is an existing reference: */
+	prefix = xmalloc(prefixlen + 2);
+	memcpy(prefix, refname, prefixlen + 1);
+	for (slash = strchr(prefix, '/'); slash; slash = strchr(slash + 1, '/')) {
+		*slash = '\0';
+		if (strcmp(oldrefname, prefix)) {
+			struct ref_entry *entry = find_ref(direntry, prefix);
+			if (entry) {
+				error("'%s' exists; cannot create '%s'", prefix, refname);
+				free(prefix);
+				return 0;
+			}
+		}
+		*slash = '/';
+	}
+
+	/* Check whether refname is a proper prefix of an existing reference: */
+	prefix[prefixlen++] = '/';
+	prefix[prefixlen] = '\0';
 	data.oldrefname = oldrefname;
 	data.conflicting_refname = NULL;
 
-	assert(direntry->flag & REF_DIR);
+	direntry = find_containing_direntry(direntry, prefix, 0);
+	if (!direntry)
+		return 1;
 
 	if (do_for_each_ref_in_dir(direntry, 0, "", name_conflict_fn,
 				   0, DO_FOR_EACH_INCLUDE_BROKEN,
-- 
1.7.8

  parent reply	other threads:[~2011-12-12  5:41 UTC|newest]

Thread overview: 79+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-12-12  5:38 [PATCH v2 00/51] ref-api-C and ref-api-D re-roll mhagger
2011-12-12  5:38 ` [PATCH v2 01/51] struct ref_entry: document name member mhagger
2011-12-12  5:38 ` [PATCH v2 02/51] refs: rename "refname" variables mhagger
2011-12-13  0:37   ` Junio C Hamano
2011-12-12  5:38 ` [PATCH v2 03/51] refs: rename parameters result -> sha1 mhagger
2011-12-12  5:38 ` [PATCH v2 04/51] clear_ref_array(): rename from free_ref_array() mhagger
2011-12-12  5:38 ` [PATCH v2 05/51] is_refname_available(): remove the "quiet" argument mhagger
2011-12-12  5:38 ` [PATCH v2 06/51] parse_ref_line(): add docstring mhagger
2011-12-12  5:38 ` [PATCH v2 07/51] add_ref(): " mhagger
2011-12-12  5:38 ` [PATCH v2 08/51] is_dup_ref(): extract function from sort_ref_array() mhagger
2011-12-12  8:33   ` Jeff King
2011-12-12 11:44     ` Michael Haggerty
2011-12-12 17:14       ` Junio C Hamano
2011-12-12 22:33   ` Junio C Hamano
2011-12-13  4:35     ` Michael Haggerty
2011-12-13  5:00       ` Michael Haggerty
2011-12-12  5:38 ` [PATCH v2 09/51] refs: change signatures of get_packed_refs() and get_loose_refs() mhagger
2011-12-12  5:38 ` [PATCH v2 10/51] get_ref_dir(): change signature mhagger
2011-12-12  5:38 ` [PATCH v2 11/51] resolve_gitlink_ref(): improve docstring mhagger
2011-12-12  5:38 ` [PATCH v2 12/51] Pass a (ref_cache *) to the resolve_gitlink_*() helper functions mhagger
2011-12-12  5:38 ` [PATCH v2 13/51] resolve_gitlink_ref_recursive(): change to work with struct ref_cache mhagger
2011-12-12  5:38 ` [PATCH v2 14/51] repack_without_ref(): remove temporary mhagger
2011-12-12  5:38 ` [PATCH v2 15/51] create_ref_entry(): extract function from add_ref() mhagger
2011-12-12  5:38 ` [PATCH v2 16/51] add_ref(): take a (struct ref_entry *) parameter mhagger
2011-12-12  5:38 ` [PATCH v2 17/51] do_for_each_ref(): correctly terminate while processesing extra_refs mhagger
2011-12-12 22:41   ` Junio C Hamano
2011-12-12  5:38 ` [PATCH v2 18/51] do_for_each_ref_in_array(): new function mhagger
2011-12-12  5:38 ` [PATCH v2 19/51] do_for_each_ref_in_arrays(): " mhagger
2011-12-12  5:38 ` [PATCH v2 20/51] repack_without_ref(): reimplement using do_for_each_ref_in_array() mhagger
2011-12-12 22:44   ` Junio C Hamano
2011-12-12  5:38 ` [PATCH v2 21/51] names_conflict(): new function, extracted from is_refname_available() mhagger
2011-12-12  5:38 ` [PATCH v2 22/51] names_conflict(): simplify implementation mhagger
2011-12-12  5:38 ` [PATCH v2 23/51] is_refname_available(): reimplement using do_for_each_ref_in_array() mhagger
2011-12-12  5:38 ` [PATCH v2 24/51] refs.c: reorder definitions more logically mhagger
2011-12-12  5:38 ` [PATCH v2 25/51] free_ref_entry(): new function mhagger
2011-12-12  5:38 ` [PATCH v2 26/51] check_refname_component(): return 0 for zero-length components mhagger
2011-12-12  5:38 ` [PATCH v2 27/51] struct ref_entry: nest the value part in a union mhagger
2011-12-12  5:38 ` [PATCH v2 28/51] refs.c: rename ref_array -> ref_dir mhagger
2011-12-13  0:45   ` Junio C Hamano
2011-12-13  5:43     ` Michael Haggerty
2011-12-13  6:37       ` Junio C Hamano
2011-12-13 19:12         ` Michael Haggerty
2011-12-13 19:17           ` Junio C Hamano
2011-12-13 22:13           ` Michael Haggerty
2011-12-13 23:24             ` Junio C Hamano
2011-12-14  0:19               ` Junio C Hamano
2011-12-14  2:33                 ` Jeff King
2011-12-15  8:19                   ` Michael Haggerty
2011-12-15  8:37                     ` Jeff King
2012-01-17 15:07               ` Michael Haggerty
2012-02-10 14:51                 ` Michael Haggerty
2012-02-10 20:44                   ` Jeff King
2012-02-10 21:17                     ` Junio C Hamano
2012-02-11  6:33                       ` Michael Haggerty
2011-12-12  5:38 ` [PATCH v2 29/51] refs: store references hierarchically mhagger
2011-12-12  5:38 ` [PATCH v2 30/51] sort_ref_dir(): do not sort if already sorted mhagger
2011-12-12 23:26   ` Junio C Hamano
2011-12-12  5:38 ` [PATCH v2 31/51] refs: sort ref_dirs lazily mhagger
2011-12-12  5:38 ` [PATCH v2 32/51] do_for_each_ref(): only iterate over the subtree that was requested mhagger
2011-12-12  5:38 ` [PATCH v2 33/51] get_ref_dir(): keep track of the current ref_dir mhagger
2011-12-12  5:38 ` [PATCH v2 34/51] refs: wrap top-level ref_dirs in ref_entries mhagger
2011-12-12  5:38 ` [PATCH v2 35/51] get_packed_refs(): return (ref_entry *) instead of (ref_dir *) mhagger
2011-12-12  5:38 ` [PATCH v2 36/51] get_loose_refs(): " mhagger
2011-12-12  5:38 ` [PATCH v2 37/51] is_refname_available(): take " mhagger
2011-12-12  5:38 ` [PATCH v2 38/51] find_ref(): " mhagger
2011-12-12  5:38 ` [PATCH v2 39/51] read_packed_refs(): " mhagger
2011-12-12  5:38 ` [PATCH v2 40/51] add_ref(): " mhagger
2011-12-12  5:38 ` [PATCH v2 41/51] find_containing_direntry(): use " mhagger
2011-12-12  5:38 ` [PATCH v2 42/51] search_ref_dir(): take " mhagger
2011-12-12  5:38 ` [PATCH v2 43/51] add_entry(): " mhagger
2011-12-12  5:38 ` [PATCH v2 44/51] do_for_each_ref_in_dir*(): " mhagger
2011-12-12  5:38 ` [PATCH v2 45/51] sort_ref_dir(): " mhagger
2011-12-12  5:38 ` [PATCH v2 46/51] struct ref_dir: store a reference to the enclosing ref_cache mhagger
2011-12-12  5:38 ` [PATCH v2 47/51] read_loose_refs(): take a (ref_entry *) as argument mhagger
2011-12-12  5:38 ` [PATCH v2 48/51] refs: read loose references lazily mhagger
2011-12-12  5:38 ` mhagger [this message]
2011-12-12  5:38 ` [PATCH v2 50/51] read_packed_refs(): keep track of the directory being worked in mhagger
2011-12-12  5:38 ` [PATCH v2 51/51] repack_without_ref(): call clear_packed_ref_cache() mhagger
2011-12-12  8:24 ` [PATCH v2 00/51] ref-api-C and ref-api-D re-roll Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1323668338-1764-50-git-send-email-mhagger@alum.mit.edu \
    --to=mhagger@alum.mit.edu \
    --cc=drew.northup@maine.edu \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=hvoigt@hvoigt.net \
    --cc=jnareb@gmail.com \
    --cc=johan@herland.net \
    --cc=julian@quantumfyre.co.uk \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).