git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 00/12] Use refs API more consistently
@ 2011-10-28 11:27 mhagger
  2011-10-28 11:27 ` [PATCH v2 01/12] Rename another local variable name -> refname mhagger
                   ` (11 more replies)
  0 siblings, 12 replies; 13+ messages in thread
From: mhagger @ 2011-10-28 11:27 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
	Johan Herland, Julian Phillips, Michael Haggerty

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

This is a re-roll of the patch series on top of v3 of "Tidying up
references code", which in turn applies to gitster/master.

This series conflicts with the "Checking full vs. partial refnames"
series but not in a fundamental way.  Unless there is an important
reason to merge the latter to master before this patch series, I
suggest that I reroll "Checking full vs. partial refnames" on top of
these ref-api changes after they hit master.

This patch series primarily has to do with using the refs API more
consistently within refs.c itself.  We want to minimize the surface
area for accessing the ref_cache data structure so that when it is
changed into a tree (we're on the verge of that change now, I
promise!) we don't have to change callers more than necessary.  There
is also a bit of an eat-your-own-dogfood thing going on here; if
for_each_ref() and its friends are good enough for "outsiders", they
should be good enough for most internal use.

The first two patches are trivial preparation cleanups.

The third verifies that refnames read from packed-ref files are
properly formatted.  When the REFNAME_FULL changes become available,
this check can be tightened up.

The do_for_each_ref_in_{array,arrays}() functions provide an
iterator-like interface to iterating over a single ref_array and
iterating over two ref_arrays in parallel.  These are useful
abstractions in and of themselves (the former is used to reimplement
repack_without_ref() and is_refname_available() in patches 9 and 12
respectively).  And they will be even more useful when references are
stored hierarchically.

Michael Haggerty (12):
  Rename another local variable name -> refname
  repack_without_ref(): remove temporary
  parse_ref_line(): add a check that the refname is properly formatted
  create_ref_entry(): extract function from add_ref()
  add_ref(): take a (struct ref_entry *) parameter
  do_for_each_ref(): correctly terminate while processesing extra_refs
  do_for_each_ref_in_array(): new function
  do_for_each_ref_in_arrays(): new function
  repack_without_ref(): reimplement using do_for_each_ref_in_array()
  names_conflict(): new function, extracted from is_refname_available()
  names_conflict(): simplify implementation
  is_refname_available(): reimplement using do_for_each_ref_in_array()

 refs.c |  255 ++++++++++++++++++++++++++++++++++++++++------------------------
 1 files changed, 160 insertions(+), 95 deletions(-)

-- 
1.7.7

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v2 01/12] Rename another local variable name -> refname
  2011-10-28 11:27 [PATCH v2 00/12] Use refs API more consistently mhagger
@ 2011-10-28 11:27 ` mhagger
  2011-10-28 11:27 ` [PATCH v2 02/12] repack_without_ref(): remove temporary mhagger
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: mhagger @ 2011-10-28 11:27 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
	Johan Herland, Julian Phillips, Michael Haggerty

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


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

diff --git a/refs.c b/refs.c
index 1c6de61..855c791 100644
--- a/refs.c
+++ b/refs.c
@@ -316,11 +316,11 @@ static void get_ref_dir(struct ref_cache *refs, const char *base,
 	if (dir) {
 		struct dirent *de;
 		int baselen = strlen(base);
-		char *ref = xmalloc(baselen + 257);
+		char *refname = xmalloc(baselen + 257);
 
-		memcpy(ref, base, baselen);
+		memcpy(refname, base, baselen);
 		if (baselen && base[baselen-1] != '/')
-			ref[baselen++] = '/';
+			refname[baselen++] = '/';
 
 		while ((de = readdir(dir)) != NULL) {
 			unsigned char sha1[20];
@@ -336,31 +336,31 @@ static void get_ref_dir(struct ref_cache *refs, const char *base,
 				continue;
 			if (has_extension(de->d_name, ".lock"))
 				continue;
-			memcpy(ref + baselen, de->d_name, namelen+1);
+			memcpy(refname + baselen, de->d_name, namelen+1);
 			refdir = *refs->name
-				? git_path_submodule(refs->name, "%s", ref)
-				: git_path("%s", ref);
+				? git_path_submodule(refs->name, "%s", refname)
+				: git_path("%s", refname);
 			if (stat(refdir, &st) < 0)
 				continue;
 			if (S_ISDIR(st.st_mode)) {
-				get_ref_dir(refs, ref, array);
+				get_ref_dir(refs, refname, array);
 				continue;
 			}
 			if (*refs->name) {
 				hashclr(sha1);
 				flag = 0;
-				if (resolve_gitlink_ref(refs->name, ref, sha1) < 0) {
+				if (resolve_gitlink_ref(refs->name, refname, sha1) < 0) {
 					hashclr(sha1);
 					flag |= REF_ISBROKEN;
 				}
 			} else
-				if (!resolve_ref(ref, sha1, 1, &flag)) {
+				if (!resolve_ref(refname, sha1, 1, &flag)) {
 					hashclr(sha1);
 					flag |= REF_ISBROKEN;
 				}
-			add_ref(ref, sha1, flag, array, NULL);
+			add_ref(refname, sha1, flag, array, NULL);
 		}
-		free(ref);
+		free(refname);
 		closedir(dir);
 	}
 }
-- 
1.7.7

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 02/12] repack_without_ref(): remove temporary
  2011-10-28 11:27 [PATCH v2 00/12] Use refs API more consistently mhagger
  2011-10-28 11:27 ` [PATCH v2 01/12] Rename another local variable name -> refname mhagger
@ 2011-10-28 11:27 ` mhagger
  2011-10-28 11:27 ` [PATCH v2 03/12] parse_ref_line(): add a check that the refname is properly formatted mhagger
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: mhagger @ 2011-10-28 11:27 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
	Johan Herland, Julian Phillips, Michael Haggerty

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


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

diff --git a/refs.c b/refs.c
index 855c791..7191437 100644
--- a/refs.c
+++ b/refs.c
@@ -1282,12 +1282,10 @@ static struct lock_file packlock;
 static int repack_without_ref(const char *refname)
 {
 	struct ref_array *packed;
-	struct ref_entry *ref;
 	int fd, i;
 
 	packed = get_packed_refs(get_ref_cache(NULL));
-	ref = search_ref_array(packed, refname);
-	if (ref == NULL)
+	if (search_ref_array(packed, refname) == NULL)
 		return 0;
 	fd = hold_lock_file_for_update(&packlock, git_path("packed-refs"), 0);
 	if (fd < 0) {
@@ -1298,8 +1296,7 @@ static int repack_without_ref(const char *refname)
 	for (i = 0; i < packed->nr; i++) {
 		char line[PATH_MAX + 100];
 		int len;
-
-		ref = packed->refs[i];
+		struct ref_entry *ref = packed->refs[i];
 
 		if (!strcmp(refname, ref->name))
 			continue;
-- 
1.7.7

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 03/12] parse_ref_line(): add a check that the refname is properly formatted
  2011-10-28 11:27 [PATCH v2 00/12] Use refs API more consistently mhagger
  2011-10-28 11:27 ` [PATCH v2 01/12] Rename another local variable name -> refname mhagger
  2011-10-28 11:27 ` [PATCH v2 02/12] repack_without_ref(): remove temporary mhagger
@ 2011-10-28 11:27 ` mhagger
  2011-10-28 11:27 ` [PATCH v2 04/12] create_ref_entry(): extract function from add_ref() mhagger
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: mhagger @ 2011-10-28 11:27 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
	Johan Herland, Julian Phillips, Michael Haggerty

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


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

diff --git a/refs.c b/refs.c
index 7191437..080e277 100644
--- a/refs.c
+++ b/refs.c
@@ -50,6 +50,9 @@ static const char *parse_ref_line(char *line, unsigned char *sha1)
 		return NULL;
 	line[len] = 0;
 
+	if (check_refname_format(line, REFNAME_ALLOW_ONELEVEL))
+		return NULL;
+
 	return line;
 }
 
-- 
1.7.7

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 04/12] create_ref_entry(): extract function from add_ref()
  2011-10-28 11:27 [PATCH v2 00/12] Use refs API more consistently mhagger
                   ` (2 preceding siblings ...)
  2011-10-28 11:27 ` [PATCH v2 03/12] parse_ref_line(): add a check that the refname is properly formatted mhagger
@ 2011-10-28 11:27 ` mhagger
  2011-10-28 11:27 ` [PATCH v2 05/12] add_ref(): take a (struct ref_entry *) parameter mhagger
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: mhagger @ 2011-10-28 11:27 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
	Johan Herland, Julian Phillips, Michael Haggerty

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

Separate the creation of the ref_entry from its addition to a ref_array.

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

diff --git a/refs.c b/refs.c
index 080e277..163ce91 100644
--- a/refs.c
+++ b/refs.c
@@ -56,27 +56,33 @@ static const char *parse_ref_line(char *line, unsigned char *sha1)
 	return line;
 }
 
-/* Add a ref_entry to the end of the ref_array (unsorted). */
-static void add_ref(const char *refname, const unsigned char *sha1,
-		    int flag, struct ref_array *refs,
-		    struct ref_entry **new_entry)
+static struct ref_entry *create_ref_entry(const char *refname,
+					  const unsigned char *sha1, int flag)
 {
 	int len;
-	struct ref_entry *entry;
+	struct ref_entry *ref;
 
-	/* Allocate it and add it in.. */
-	len = strlen(refname) + 1;
-	entry = xmalloc(sizeof(struct ref_entry) + len);
-	hashcpy(entry->sha1, sha1);
-	hashclr(entry->peeled);
 	if (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL|REFNAME_DOT_COMPONENT))
 		die("Reference has invalid format: '%s'", refname);
-	memcpy(entry->name, refname, len);
-	entry->flag = flag;
-	if (new_entry)
-		*new_entry = entry;
+	len = strlen(refname) + 1;
+	ref = xmalloc(sizeof(struct ref_entry) + len);
+	hashcpy(ref->sha1, sha1);
+	hashclr(ref->peeled);
+	memcpy(ref->name, refname, len);
+	ref->flag = flag;
+	return ref;
+}
+
+/* Add a ref_entry to the end of the ref_array (unsorted). */
+static void add_ref(const char *refname, const unsigned char *sha1,
+		    int flag, struct ref_array *refs,
+		    struct ref_entry **new_ref)
+{
+	struct ref_entry *ref = create_ref_entry(refname, sha1, flag);
+	if (new_ref)
+		*new_ref = ref;
 	ALLOC_GROW(refs->refs, refs->nr + 1, refs->alloc);
-	refs->refs[refs->nr++] = entry;
+	refs->refs[refs->nr++] = ref;
 }
 
 static int ref_entry_cmp(const void *a, const void *b)
-- 
1.7.7

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 05/12] add_ref(): take a (struct ref_entry *) parameter
  2011-10-28 11:27 [PATCH v2 00/12] Use refs API more consistently mhagger
                   ` (3 preceding siblings ...)
  2011-10-28 11:27 ` [PATCH v2 04/12] create_ref_entry(): extract function from add_ref() mhagger
@ 2011-10-28 11:27 ` mhagger
  2011-10-28 11:27 ` [PATCH v2 06/12] do_for_each_ref(): correctly terminate while processesing extra_refs mhagger
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: mhagger @ 2011-10-28 11:27 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
	Johan Herland, Julian Phillips, Michael Haggerty

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

Take a pointer to the ref_entry to add to the array, rather than
creating the ref_entry within the function.  This opens the way to
having multiple kinds of ref_entries.

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

diff --git a/refs.c b/refs.c
index 163ce91..c76d8b5 100644
--- a/refs.c
+++ b/refs.c
@@ -74,13 +74,8 @@ static struct ref_entry *create_ref_entry(const char *refname,
 }
 
 /* Add a ref_entry to the end of the ref_array (unsorted). */
-static void add_ref(const char *refname, const unsigned char *sha1,
-		    int flag, struct ref_array *refs,
-		    struct ref_entry **new_ref)
+static void add_ref(struct ref_array *refs, struct ref_entry *ref)
 {
-	struct ref_entry *ref = create_ref_entry(refname, sha1, flag);
-	if (new_ref)
-		*new_ref = ref;
 	ALLOC_GROW(refs->refs, refs->nr + 1, refs->alloc);
 	refs->refs[refs->nr++] = ref;
 }
@@ -265,7 +260,8 @@ static void read_packed_refs(FILE *f, struct ref_array *array)
 
 		refname = parse_ref_line(refline, sha1);
 		if (refname) {
-			add_ref(refname, sha1, flag, array, &last);
+			last = create_ref_entry(refname, sha1, flag);
+			add_ref(array, last);
 			continue;
 		}
 		if (last &&
@@ -280,7 +276,7 @@ static void read_packed_refs(FILE *f, struct ref_array *array)
 
 void add_extra_ref(const char *refname, const unsigned char *sha1, int flag)
 {
-	add_ref(refname, sha1, flag, &extra_refs, NULL);
+	add_ref(&extra_refs, create_ref_entry(refname, sha1, flag));
 }
 
 void clear_extra_refs(void)
@@ -367,7 +363,7 @@ static void get_ref_dir(struct ref_cache *refs, const char *base,
 					hashclr(sha1);
 					flag |= REF_ISBROKEN;
 				}
-			add_ref(refname, sha1, flag, array, NULL);
+			add_ref(array, create_ref_entry(refname, sha1, flag));
 		}
 		free(refname);
 		closedir(dir);
-- 
1.7.7

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 06/12] do_for_each_ref(): correctly terminate while processesing extra_refs
  2011-10-28 11:27 [PATCH v2 00/12] Use refs API more consistently mhagger
                   ` (4 preceding siblings ...)
  2011-10-28 11:27 ` [PATCH v2 05/12] add_ref(): take a (struct ref_entry *) parameter mhagger
@ 2011-10-28 11:27 ` mhagger
  2011-10-28 11:27 ` [PATCH v2 07/12] do_for_each_ref_in_array(): new function mhagger
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: mhagger @ 2011-10-28 11:27 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
	Johan Herland, Julian Phillips, Michael Haggerty

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

If the user-supplied function returns a nonzero value while processing
extra_refs, terminate without processing the rest of the list.

This probably has no practical importance, but makes the handling of
extra_refs a little bit more consistent with the handling of other
refs.

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

diff --git a/refs.c b/refs.c
index c76d8b5..8043436 100644
--- a/refs.c
+++ b/refs.c
@@ -710,8 +710,11 @@ static int do_for_each_ref(const char *submodule, const char *base, each_ref_fn
 
 	struct ref_array *extra = &extra_refs;
 
-	for (i = 0; i < extra->nr; i++)
+	for (i = 0; i < extra->nr; i++) {
 		retval = do_one_ref(base, fn, trim, flags, cb_data, extra->refs[i]);
+		if (retval)
+			goto end_each;
+	}
 
 	while (p < packed->nr && l < loose->nr) {
 		struct ref_entry *entry;
-- 
1.7.7

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 07/12] do_for_each_ref_in_array(): new function
  2011-10-28 11:27 [PATCH v2 00/12] Use refs API more consistently mhagger
                   ` (5 preceding siblings ...)
  2011-10-28 11:27 ` [PATCH v2 06/12] do_for_each_ref(): correctly terminate while processesing extra_refs mhagger
@ 2011-10-28 11:27 ` mhagger
  2011-10-28 11:28 ` [PATCH v2 08/12] do_for_each_ref_in_arrays(): " mhagger
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: mhagger @ 2011-10-28 11:27 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
	Johan Herland, Julian Phillips, Michael Haggerty

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

Extract function do_for_each_ref_in_array() from do_for_each_ref().

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

diff --git a/refs.c b/refs.c
index 8043436..24a7a4d 100644
--- a/refs.c
+++ b/refs.c
@@ -700,21 +700,31 @@ fallback:
 	return -1;
 }
 
+static int do_for_each_ref_in_array(struct ref_array *array, int offset,
+				    const char *base,
+				    each_ref_fn fn, int trim, int flags, void *cb_data)
+{
+	int i;
+	for (i = offset; i < array->nr; i++) {
+		int retval = do_one_ref(base, fn, trim, flags, cb_data, array->refs[i]);
+		if (retval)
+			return retval;
+	}
+	return 0;
+}
+
 static int do_for_each_ref(const char *submodule, const char *base, each_ref_fn fn,
 			   int trim, int flags, void *cb_data)
 {
-	int retval = 0, i, p = 0, l = 0;
+	int retval = 0, p = 0, l = 0;
 	struct ref_cache *refs = get_ref_cache(submodule);
 	struct ref_array *packed = get_packed_refs(refs);
 	struct ref_array *loose = get_loose_refs(refs);
 
-	struct ref_array *extra = &extra_refs;
-
-	for (i = 0; i < extra->nr; i++) {
-		retval = do_one_ref(base, fn, trim, flags, cb_data, extra->refs[i]);
-		if (retval)
-			goto end_each;
-	}
+	retval = do_for_each_ref_in_array(&extra_refs, 0,
+					  base, fn, trim, flags, cb_data);
+	if (retval)
+		goto end_each;
 
 	while (p < packed->nr && l < loose->nr) {
 		struct ref_entry *entry;
@@ -734,14 +744,11 @@ static int do_for_each_ref(const char *submodule, const char *base, each_ref_fn
 	}
 
 	if (l < loose->nr) {
-		p = l;
-		packed = loose;
-	}
-
-	for (; p < packed->nr; p++) {
-		retval = do_one_ref(base, fn, trim, flags, cb_data, packed->refs[p]);
-		if (retval)
-			goto end_each;
+		retval = do_for_each_ref_in_array(loose, l,
+						  base, fn, trim, flags, cb_data);
+	} else {
+		retval = do_for_each_ref_in_array(packed, p,
+						  base, fn, trim, flags, cb_data);
 	}
 
 end_each:
-- 
1.7.7

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 08/12] do_for_each_ref_in_arrays(): new function
  2011-10-28 11:27 [PATCH v2 00/12] Use refs API more consistently mhagger
                   ` (6 preceding siblings ...)
  2011-10-28 11:27 ` [PATCH v2 07/12] do_for_each_ref_in_array(): new function mhagger
@ 2011-10-28 11:28 ` mhagger
  2011-10-28 11:28 ` [PATCH v2 09/12] repack_without_ref(): reimplement using do_for_each_ref_in_array() mhagger
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: mhagger @ 2011-10-28 11:28 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
	Johan Herland, Julian Phillips, Michael Haggerty

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

Extract function do_for_each_ref_in_arrays() from do_for_each_ref().

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

diff --git a/refs.c b/refs.c
index 24a7a4d..ad7538a 100644
--- a/refs.c
+++ b/refs.c
@@ -713,45 +713,58 @@ static int do_for_each_ref_in_array(struct ref_array *array, int offset,
 	return 0;
 }
 
-static int do_for_each_ref(const char *submodule, const char *base, each_ref_fn fn,
-			   int trim, int flags, void *cb_data)
+static int do_for_each_ref_in_arrays(struct ref_array *array1,
+				     struct ref_array *array2,
+				     const char *base, each_ref_fn fn, int trim,
+				     int flags, void *cb_data)
 {
-	int retval = 0, p = 0, l = 0;
-	struct ref_cache *refs = get_ref_cache(submodule);
-	struct ref_array *packed = get_packed_refs(refs);
-	struct ref_array *loose = get_loose_refs(refs);
-
-	retval = do_for_each_ref_in_array(&extra_refs, 0,
-					  base, fn, trim, flags, cb_data);
-	if (retval)
-		goto end_each;
+	int retval;
+	int i1 = 0, i2 = 0;
 
-	while (p < packed->nr && l < loose->nr) {
-		struct ref_entry *entry;
-		int cmp = strcmp(packed->refs[p]->name, loose->refs[l]->name);
-		if (!cmp) {
-			p++;
+	while (1) {
+		struct ref_entry *e1, *e2;
+		int cmp;
+		if (i1 == array1->nr) {
+			return do_for_each_ref_in_array(array2, i2,
+							base, fn, trim, flags, cb_data);
+		}
+		if (i2 == array2->nr) {
+			return do_for_each_ref_in_array(array1, i1,
+							base, fn, trim, flags, cb_data);
+		}
+		e1 = array1->refs[i1];
+		e2 = array2->refs[i2];
+		cmp = strcmp(e1->name, e2->name);
+		if (cmp == 0) {
+			/* Two refs with the same name; ignore the one from array1. */
+			i1++;
 			continue;
 		}
-		if (cmp > 0) {
-			entry = loose->refs[l++];
+		if (cmp < 0) {
+			retval = do_one_ref(base, fn, trim, flags, cb_data, e1);
+			i1++;
 		} else {
-			entry = packed->refs[p++];
+			retval = do_one_ref(base, fn, trim, flags, cb_data, e2);
+			i2++;
 		}
-		retval = do_one_ref(base, fn, trim, flags, cb_data, entry);
 		if (retval)
-			goto end_each;
+			return retval;
 	}
+}
 
-	if (l < loose->nr) {
-		retval = do_for_each_ref_in_array(loose, l,
-						  base, fn, trim, flags, cb_data);
-	} else {
-		retval = do_for_each_ref_in_array(packed, p,
-						  base, fn, trim, flags, cb_data);
-	}
+static int do_for_each_ref(const char *submodule, const char *base, each_ref_fn fn,
+			   int trim, int flags, void *cb_data)
+{
+	int retval = 0;
+	struct ref_cache *refs = get_ref_cache(submodule);
+
+	retval = do_for_each_ref_in_array(&extra_refs, 0,
+					  base, fn, trim, flags, cb_data);
+	if (!retval)
+		retval = do_for_each_ref_in_arrays(get_packed_refs(refs),
+						   get_loose_refs(refs),
+						   base, fn, trim, flags, cb_data);
 
-end_each:
 	current_ref = NULL;
 	return retval;
 }
-- 
1.7.7

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 09/12] repack_without_ref(): reimplement using do_for_each_ref_in_array()
  2011-10-28 11:27 [PATCH v2 00/12] Use refs API more consistently mhagger
                   ` (7 preceding siblings ...)
  2011-10-28 11:28 ` [PATCH v2 08/12] do_for_each_ref_in_arrays(): " mhagger
@ 2011-10-28 11:28 ` mhagger
  2011-10-28 11:28 ` [PATCH v2 10/12] names_conflict(): new function, extracted from is_refname_available() mhagger
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: mhagger @ 2011-10-28 11:28 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
	Johan Herland, Julian Phillips, Michael Haggerty

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


It costs a bit of boilerplate, but it means that the function can be
ignorant of how cached refs are stored.

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

diff --git a/refs.c b/refs.c
index ad7538a..d56f265 100644
--- a/refs.c
+++ b/refs.c
@@ -1305,36 +1305,46 @@ struct ref_lock *lock_any_ref_for_update(const char *refname,
 	return lock_ref_sha1_basic(refname, old_sha1, flags, NULL);
 }
 
+struct repack_without_ref_sb {
+	const char *refname;
+	int fd;
+};
+
+static int repack_without_ref_fn(const char *refname, const unsigned char *sha1,
+				 int flags, void *cb_data)
+{
+	struct repack_without_ref_sb *data = cb_data;
+	char line[PATH_MAX + 100];
+	int len;
+
+	if (!strcmp(data->refname, refname))
+		return 0;
+	len = snprintf(line, sizeof(line), "%s %s\n",
+		       sha1_to_hex(sha1), refname);
+	/* this should not happen but just being defensive */
+	if (len > sizeof(line))
+		die("too long a refname '%s'", refname);
+	write_or_die(data->fd, line, len);
+	return 0;
+}
+
 static struct lock_file packlock;
 
 static int repack_without_ref(const char *refname)
 {
+	struct repack_without_ref_sb data;
 	struct ref_array *packed;
-	int fd, i;
 
 	packed = get_packed_refs(get_ref_cache(NULL));
 	if (search_ref_array(packed, refname) == NULL)
 		return 0;
-	fd = hold_lock_file_for_update(&packlock, git_path("packed-refs"), 0);
-	if (fd < 0) {
+	data.refname = refname;
+	data.fd = hold_lock_file_for_update(&packlock, git_path("packed-refs"), 0);
+	if (data.fd < 0) {
 		unable_to_lock_error(git_path("packed-refs"), errno);
 		return error("cannot delete '%s' from packed refs", refname);
 	}
-
-	for (i = 0; i < packed->nr; i++) {
-		char line[PATH_MAX + 100];
-		int len;
-		struct ref_entry *ref = packed->refs[i];
-
-		if (!strcmp(refname, ref->name))
-			continue;
-		len = snprintf(line, sizeof(line), "%s %s\n",
-			       sha1_to_hex(ref->sha1), ref->name);
-		/* this should not happen but just being defensive */
-		if (len > sizeof(line))
-			die("too long a refname '%s'", ref->name);
-		write_or_die(fd, line, len);
-	}
+	do_for_each_ref_in_array(packed, 0, "", repack_without_ref_fn, 0, 0, &data);
 	return commit_lock_file(&packlock);
 }
 
-- 
1.7.7

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 10/12] names_conflict(): new function, extracted from is_refname_available()
  2011-10-28 11:27 [PATCH v2 00/12] Use refs API more consistently mhagger
                   ` (8 preceding siblings ...)
  2011-10-28 11:28 ` [PATCH v2 09/12] repack_without_ref(): reimplement using do_for_each_ref_in_array() mhagger
@ 2011-10-28 11:28 ` mhagger
  2011-10-28 11:28 ` [PATCH v2 11/12] names_conflict(): simplify implementation mhagger
  2011-10-28 11:28 ` [PATCH v2 12/12] is_refname_available(): reimplement using do_for_each_ref_in_array() mhagger
  11 siblings, 0 replies; 13+ messages in thread
From: mhagger @ 2011-10-28 11:28 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
	Johan Herland, Julian Phillips, Michael Haggerty

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


This costs an extra strlen() in the loop, but even that small price
will be clawed back in the next patch.

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

diff --git a/refs.c b/refs.c
index d56f265..62e5fd3 100644
--- a/refs.c
+++ b/refs.c
@@ -1092,6 +1092,30 @@ static int remove_empty_directories(const char *file)
 }
 
 /*
+ * 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)
+{
+	int len1 = strlen(refname1);
+	int len2 = strlen(refname2);
+	int cmplen;
+	const char *lead;
+
+	if (len1 < len2) {
+		cmplen = len1;
+		lead = refname2;
+	} else {
+		cmplen = len2;
+		lead = refname1;
+	}
+	return !strncmp(refname1, refname2, cmplen) && lead[cmplen] == '/';
+}
+
+/*
  * 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.,
@@ -1101,20 +1125,15 @@ static int remove_empty_directories(const char *file)
 static int is_refname_available(const char *refname, const char *oldrefname,
 				struct ref_array *array)
 {
-	int i, namlen = strlen(refname); /* e.g. 'foo/bar' */
+	int i;
 	for (i = 0; i < array->nr; i++ ) {
 		struct ref_entry *entry = array->refs[i];
-		/* entry->name could be 'foo' or 'foo/bar/baz' */
-		if (!oldrefname || strcmp(oldrefname, entry->name)) {
-			int len = strlen(entry->name);
-			int cmplen = (namlen < len) ? namlen : len;
-			const char *lead = (namlen < len) ? entry->name : refname;
-			if (!strncmp(refname, entry->name, cmplen) &&
-			    lead[cmplen] == '/') {
-				error("'%s' exists; cannot create '%s'",
-				      entry->name, refname);
-				return 0;
-			}
+		if (oldrefname && !strcmp(oldrefname, entry->name))
+			continue;
+		if (names_conflict(refname, entry->name)) {
+			error("'%s' exists; cannot create '%s'",
+			      entry->name, refname);
+			return 0;
 		}
 	}
 	return 1;
-- 
1.7.7

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 11/12] names_conflict(): simplify implementation
  2011-10-28 11:27 [PATCH v2 00/12] Use refs API more consistently mhagger
                   ` (9 preceding siblings ...)
  2011-10-28 11:28 ` [PATCH v2 10/12] names_conflict(): new function, extracted from is_refname_available() mhagger
@ 2011-10-28 11:28 ` mhagger
  2011-10-28 11:28 ` [PATCH v2 12/12] is_refname_available(): reimplement using do_for_each_ref_in_array() mhagger
  11 siblings, 0 replies; 13+ messages in thread
From: mhagger @ 2011-10-28 11:28 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
	Johan Herland, Julian Phillips, Michael Haggerty

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


Save a bunch of lines of code and a couple of strlen() calls.

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

diff --git a/refs.c b/refs.c
index 62e5fd3..2a5d8b2 100644
--- a/refs.c
+++ b/refs.c
@@ -1100,19 +1100,10 @@ static int remove_empty_directories(const char *file)
  */
 static int names_conflict(const char *refname1, const char *refname2)
 {
-	int len1 = strlen(refname1);
-	int len2 = strlen(refname2);
-	int cmplen;
-	const char *lead;
-
-	if (len1 < len2) {
-		cmplen = len1;
-		lead = refname2;
-	} else {
-		cmplen = len2;
-		lead = refname1;
-	}
-	return !strncmp(refname1, refname2, cmplen) && lead[cmplen] == '/';
+	for (; *refname1 && *refname1 == *refname2; refname1++, refname2++)
+		;
+	return (*refname1 == '\0' && *refname2 == '/')
+		|| (*refname1 == '/' && *refname2 == '\0');
 }
 
 /*
-- 
1.7.7

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 12/12] is_refname_available(): reimplement using do_for_each_ref_in_array()
  2011-10-28 11:27 [PATCH v2 00/12] Use refs API more consistently mhagger
                   ` (10 preceding siblings ...)
  2011-10-28 11:28 ` [PATCH v2 11/12] names_conflict(): simplify implementation mhagger
@ 2011-10-28 11:28 ` mhagger
  11 siblings, 0 replies; 13+ messages in thread
From: mhagger @ 2011-10-28 11:28 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
	Johan Herland, Julian Phillips, Michael Haggerty

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

This implementation will survive upcoming changes to the ref_array
data structure.

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

diff --git a/refs.c b/refs.c
index 2a5d8b2..96466f3 100644
--- a/refs.c
+++ b/refs.c
@@ -1106,6 +1106,25 @@ static int names_conflict(const char *refname1, const char *refname2)
 		|| (*refname1 == '/' && *refname2 == '\0');
 }
 
+struct name_conflict_cb {
+	const char *refname;
+	const char *oldrefname;
+	const char *conflicting_refname;
+};
+
+static int name_conflict_fn(const char *existingrefname, const unsigned char *sha1,
+			    int flags, void *cb_data)
+{
+	struct name_conflict_cb *data = (struct name_conflict_cb *)cb_data;
+	if (data->oldrefname && !strcmp(data->oldrefname, existingrefname))
+		return 0;
+	if (names_conflict(data->refname, existingrefname)) {
+		data->conflicting_refname = existingrefname;
+		return 1;
+	}
+	return 0;
+}
+
 /*
  * Return true iff a reference named refname could be created without
  * conflicting with the name of an existing reference.  If oldrefname
@@ -1116,16 +1135,17 @@ static int names_conflict(const char *refname1, const char *refname2)
 static int is_refname_available(const char *refname, const char *oldrefname,
 				struct ref_array *array)
 {
-	int i;
-	for (i = 0; i < array->nr; i++ ) {
-		struct ref_entry *entry = array->refs[i];
-		if (oldrefname && !strcmp(oldrefname, entry->name))
-			continue;
-		if (names_conflict(refname, entry->name)) {
-			error("'%s' exists; cannot create '%s'",
-			      entry->name, refname);
-			return 0;
-		}
+	struct name_conflict_cb data;
+	data.refname = refname;
+	data.oldrefname = oldrefname;
+	data.conflicting_refname = NULL;
+
+	if (do_for_each_ref_in_array(array, 0, "", name_conflict_fn,
+				     0, DO_FOR_EACH_INCLUDE_BROKEN,
+				     &data)) {
+		error("'%s' exists; cannot create '%s'",
+		      data.conflicting_refname, refname);
+		return 0;
 	}
 	return 1;
 }
-- 
1.7.7

^ permalink raw reply related	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2011-10-28 11:32 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-10-28 11:27 [PATCH v2 00/12] Use refs API more consistently mhagger
2011-10-28 11:27 ` [PATCH v2 01/12] Rename another local variable name -> refname mhagger
2011-10-28 11:27 ` [PATCH v2 02/12] repack_without_ref(): remove temporary mhagger
2011-10-28 11:27 ` [PATCH v2 03/12] parse_ref_line(): add a check that the refname is properly formatted mhagger
2011-10-28 11:27 ` [PATCH v2 04/12] create_ref_entry(): extract function from add_ref() mhagger
2011-10-28 11:27 ` [PATCH v2 05/12] add_ref(): take a (struct ref_entry *) parameter mhagger
2011-10-28 11:27 ` [PATCH v2 06/12] do_for_each_ref(): correctly terminate while processesing extra_refs mhagger
2011-10-28 11:27 ` [PATCH v2 07/12] do_for_each_ref_in_array(): new function mhagger
2011-10-28 11:28 ` [PATCH v2 08/12] do_for_each_ref_in_arrays(): " mhagger
2011-10-28 11:28 ` [PATCH v2 09/12] repack_without_ref(): reimplement using do_for_each_ref_in_array() mhagger
2011-10-28 11:28 ` [PATCH v2 10/12] names_conflict(): new function, extracted from is_refname_available() mhagger
2011-10-28 11:28 ` [PATCH v2 11/12] names_conflict(): simplify implementation mhagger
2011-10-28 11:28 ` [PATCH v2 12/12] is_refname_available(): reimplement using do_for_each_ref_in_array() mhagger

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).