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] Fix "is_refname_available(): query only possibly-conflicting references"
Date: Tue, 15 Nov 2011 06:55:25 +0100 [thread overview]
Message-ID: <1321336525-19374-1-git-send-email-mhagger@alum.mit.edu> (raw)
In-Reply-To: <1319804921-17545-27-git-send-email-mhagger@alum.mit.edu>
From: Michael Haggerty <mhagger@alum.mit.edu>
The above-named commit didn't do all that its commit message claimed.
The problem is that do_for_each_ref_in_dir() doesn't avoid iterating
through reference subtrees outside of "prefix"; it only skips passing
those references to the callback function. So the function
unnecessarily caused all loose references to be loaded rather than
just those in the required subtree.
So instead, explicitly select the possibly-conflicting subtree and
pass it to do_for_each_ref_in_dir().
Also, simplify name_conflict_fn(). Since it 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>
---
This patch can be squashed on top of "is_refname_available(): query
only possibly-conflicting references", or applied at the end of
mh/ref-api-take-2; it does not conflict with the two commits later in
the series.
refs.c | 40 +++++++++++++++++-----------------------
1 files changed, 17 insertions(+), 23 deletions(-)
diff --git a/refs.c b/refs.c
index 6a7f9c3..b185c32 100644
--- a/refs.c
+++ b/refs.c
@@ -633,23 +633,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;
};
@@ -660,11 +644,13 @@ static int name_conflict_fn(const char *existingrefname, const unsigned char *sh
struct name_conflict_cb *data = (struct name_conflict_cb *)cb_data;
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;
}
/*
@@ -673,6 +659,11 @@ static int name_conflict_fn(const char *existingrefname, const unsigned char *sh
* 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)
@@ -706,11 +697,14 @@ static int is_refname_available(const char *refname, const char *oldrefname,
/* Check whether refname is a proper prefix of an existing reference: */
prefix[prefixlen++] = '/';
prefix[prefixlen] = '\0';
- data.refname = refname;
data.oldrefname = oldrefname;
data.conflicting_refname = NULL;
- if (do_for_each_ref_in_dir(direntry, 0, prefix, name_conflict_fn,
+ 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,
&data)) {
error("'%s' exists; cannot create '%s'",
--
1.7.7.2
next prev parent reply other threads:[~2011-11-15 5:55 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-10-28 12:28 [PATCH 00/28] Store references hierarchically in cache mhagger
2011-10-28 12:28 ` [PATCH 01/28] refs.c: reorder definitions more logically mhagger
2011-10-28 12:28 ` [PATCH 02/28] free_ref_entry(): new function mhagger
2011-10-28 12:28 ` [PATCH 03/28] check_refname_component(): return 0 for zero-length components mhagger
2011-10-28 12:28 ` [PATCH 04/28] struct ref_entry: nest the value part in a union mhagger
2011-10-28 12:28 ` [PATCH 05/28] refs.c: rename ref_array -> ref_dir mhagger
2011-10-28 12:28 ` [PATCH 06/28] refs: store references hierarchically mhagger
2011-10-28 12:28 ` [PATCH 07/28] sort_ref_dir(): do not sort if already sorted mhagger
2011-10-28 12:28 ` [PATCH 08/28] refs: sort ref_dirs lazily mhagger
2011-10-28 12:28 ` [PATCH 09/28] do_for_each_ref(): only iterate over the subtree that was requested mhagger
2011-10-28 12:28 ` [PATCH 10/28] get_ref_dir(): keep track of the current ref_dir mhagger
2011-10-28 12:28 ` [PATCH 11/28] refs: wrap top-level ref_dirs in ref_entries mhagger
2011-10-28 12:28 ` [PATCH 12/28] get_packed_refs(): return (ref_entry *) instead of (ref_dir *) mhagger
2011-10-28 12:28 ` [PATCH 13/28] get_loose_refs(): " mhagger
2011-10-28 12:28 ` [PATCH 14/28] is_refname_available(): take " mhagger
2011-10-28 12:28 ` [PATCH 15/28] find_ref(): " mhagger
2011-10-28 12:28 ` [PATCH 16/28] read_packed_refs(): " mhagger
2011-10-28 12:28 ` [PATCH 17/28] add_ref(): " mhagger
2011-10-28 12:28 ` [PATCH 18/28] find_containing_direntry(): use " mhagger
2011-10-28 12:28 ` [PATCH 19/28] search_ref_dir(): take " mhagger
2011-10-28 12:28 ` [PATCH 20/28] add_entry(): " mhagger
2011-10-28 12:28 ` [PATCH 21/28] do_for_each_ref_in_dir*(): " mhagger
2011-10-28 12:28 ` [PATCH 22/28] sort_ref_dir(): " mhagger
2011-10-28 12:28 ` [PATCH 23/28] struct ref_dir: store a reference to the enclosing ref_cache mhagger
2011-10-28 12:28 ` [PATCH 24/28] read_loose_refs(): take a (ref_entry *) as argument mhagger
2011-10-28 12:28 ` [PATCH 25/28] refs: read loose references lazily mhagger
2011-10-28 12:28 ` [PATCH 26/28] is_refname_available(): query only possibly-conflicting references mhagger
2011-11-15 5:55 ` mhagger [this message]
2011-11-15 7:24 ` [PATCH] Fix "is_refname_available(): query only possibly-conflicting references" Junio C Hamano
2011-11-15 16:19 ` Michael Haggerty
2011-11-15 19:19 ` Junio C Hamano
2011-10-28 12:28 ` [PATCH 27/28] read_packed_refs(): keep track of the directory being worked in mhagger
2011-10-28 12:28 ` [PATCH 28/28] repack_without_ref(): call clear_packed_ref_cache() mhagger
2011-10-28 13:07 ` [PATCH 00/28] Store references hierarchically in cache Ramkumar Ramachandra
2011-10-28 18:45 ` Michael Haggerty
2011-11-16 12:51 ` [PATCH 00/28] Store references hierarchically in cache -- benchmark results Michael Haggerty
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=1321336525-19374-1-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).