threads / patch / 35626

patch, 3 partsGenerate scanf_fmts more simply

Subject: [PATCH 0/3] Generate scanf_fmts more simply

## tl;dr

11 messages between Jan 8, 2014 and Jan 15, 2014. Diffs are folded; open one to read it.

replies: 10people: 2as markdown or json

Michael Haggerty· Jan 8, 2014, 14:43 UTC · lore

This is just a fun little thing that I noticed while poking around the code: the function gen_scanf_fmt() can be replaced with a simple call to snprintf().

Michael Haggerty (3):
  shorten_unambiguous_ref(): introduce a new local variable
  gen_scanf_fmt(): delete function and use snprintf() instead
  shorten_unambiguous_ref(): tighten up pointer arithmetic
 refs.c | 47 +++++++++++++++--------------------------------
 1 file changed, 15 insertions(+), 32 deletions(-)
-- 
1.8.5.2
Michael Haggerty· Jan 8, 2014, 14:43 UTC · re: Michael Haggerty · lore

[PATCH 1/3] shorten_unambiguous_ref(): introduce a new local variable

When filling the scanf_fmts array, use a separate variable to keep track of the offset to avoid clobbering total_len (which we will need in the next commit).

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 refs.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
Show changes to refs.c +4 −4
diff --git a/refs.c b/refs.c
index 3926136..9530480 100644
--- a/refs.c
+++ b/refs.c
@@ -3367,6 +3367,7 @@ char *shorten_unambiguous_ref(const char *refname, int strict)
 	/* pre generate scanf formats from ref_rev_parse_rules[] */
 	if (!nr_rules) {
 		size_t total_len = 0;
+		size_t offset = 0;
 
 		/* the rule list is NULL terminated, count them first */
 		for (nr_rules = 0; ref_rev_parse_rules[nr_rules]; nr_rules++)
@@ -3375,12 +3376,11 @@ char *shorten_unambiguous_ref(const char *refname, int strict)
 
 		scanf_fmts = xmalloc(nr_rules * sizeof(char *) + total_len);
 
-		total_len = 0;
+		offset = 0;
 		for (i = 0; i < nr_rules; i++) {
-			scanf_fmts[i] = (char *)&scanf_fmts[nr_rules]
-					+ total_len;
+			scanf_fmts[i] = (char *)&scanf_fmts[nr_rules] + offset;
 			gen_scanf_fmt(scanf_fmts[i], ref_rev_parse_rules[i]);
-			total_len += strlen(ref_rev_parse_rules[i]);
+			offset += strlen(ref_rev_parse_rules[i]);
 		}
 	}
 

-- 
1.8.5.2
Michael Haggerty· Jan 8, 2014, 14:43 UTC · re: Michael Haggerty · lore

[PATCH 2/3] gen_scanf_fmt(): delete function and use snprintf() instead

To replace "%.*s" with "%s", all we have to do is use snprintf() to interpolate "%s" into the pattern.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 refs.c | 35 +++++++++--------------------------
 1 file changed, 9 insertions(+), 26 deletions(-)
Show changes to refs.c +9 −26
diff --git a/refs.c b/refs.c
index 9530480..ef9cdea 100644
--- a/refs.c
+++ b/refs.c
@@ -3334,29 +3334,6 @@ cleanup:
 	return ret;
 }
 
-/*
- * generate a format suitable for scanf from a ref_rev_parse_rules
- * rule, that is replace the "%.*s" spec with a "%s" spec
- */
-static void gen_scanf_fmt(char *scanf_fmt, const char *rule)
-{
-	char *spec;
-
-	spec = strstr(rule, "%.*s");
-	if (!spec || strstr(spec + 4, "%.*s"))
-		die("invalid rule in ref_rev_parse_rules: %s", rule);
-
-	/* copy all until spec */
-	strncpy(scanf_fmt, rule, spec - rule);
-	scanf_fmt[spec - rule] = '\0';
-	/* copy new spec */
-	strcat(scanf_fmt, "%s");
-	/* copy remaining rule */
-	strcat(scanf_fmt, spec + 4);
-
-	return;
-}
-
 char *shorten_unambiguous_ref(const char *refname, int strict)
 {
 	int i;
@@ -3364,8 +3341,13 @@ char *shorten_unambiguous_ref(const char *refname, int strict)
 	static int nr_rules;
 	char *short_name;
 
-	/* pre generate scanf formats from ref_rev_parse_rules[] */
 	if (!nr_rules) {
+		/*
+		 * Pre-generate scanf formats from ref_rev_parse_rules[].
+		 * Generate a format suitable for scanf from a
+		 * ref_rev_parse_rules rule by interpolating "%s" at the
+		 * location of the "%.*s".
+		 */
 		size_t total_len = 0;
 		size_t offset = 0;
 
@@ -3378,9 +3360,10 @@ char *shorten_unambiguous_ref(const char *refname, int strict)
 
 		offset = 0;
 		for (i = 0; i < nr_rules; i++) {
+			assert(offset < total_len);
 			scanf_fmts[i] = (char *)&scanf_fmts[nr_rules] + offset;
-			gen_scanf_fmt(scanf_fmts[i], ref_rev_parse_rules[i]);
-			offset += strlen(ref_rev_parse_rules[i]);
+			offset += snprintf(scanf_fmts[i], total_len - offset,
+					   ref_rev_parse_rules[i], 2, "%s") + 1;
 		}
 	}
 

-- 
1.8.5.2
Junio C Hamano· Jan 9, 2014, 22:56 UTC · re: Michael Haggerty · lore

Re: [PATCH 2/3] gen_scanf_fmt(): delete function and use snprintf() instead

Michael Haggerty <mhagger@alum.mit.edu> writes:
> To replace "%.*s" with "%s", all we have to do is use snprintf()
> to interpolate "%s" into the pattern.
Cute ;-).  Thanks.
Michael Haggerty· Jan 8, 2014, 14:43 UTC · re: Michael Haggerty · lore

[PATCH 3/3] shorten_unambiguous_ref(): tighten up pointer arithmetic

As long as we're being pathologically stingy with mallocs, we might as well do the math right and save 6 (!) bytes.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
It is left to the reader to show how another 7 bytes could be saved
(11 bytes on a 64-bit architecture!)

It probably wouldn't kill performance to use a string_list here instead.

 refs.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
Show changes to refs.c +3 −3
diff --git a/refs.c b/refs.c
index ef9cdea..63b3a71 100644
--- a/refs.c
+++ b/refs.c
@@ -3351,10 +3351,10 @@ char *shorten_unambiguous_ref(const char *refname, int strict)
 		size_t total_len = 0;
 		size_t offset = 0;
 
-		/* the rule list is NULL terminated, count them first */
+		/* the rule list is NUL terminated, count them first */
 		for (nr_rules = 0; ref_rev_parse_rules[nr_rules]; nr_rules++)
-			/* no +1 because strlen("%s") < strlen("%.*s") */
-			total_len += strlen(ref_rev_parse_rules[nr_rules]);
+			/* -2 for strlen("%.*s") - strlen("%s"); +1 for NUL */
+			total_len += strlen(ref_rev_parse_rules[nr_rules]) - 2 + 1;
 
 		scanf_fmts = xmalloc(nr_rules * sizeof(char *) + total_len);
 

-- 
1.8.5.2
Junio C Hamano· Jan 9, 2014, 23:01 UTC · re: Michael Haggerty · lore

Re: [PATCH 3/3] shorten_unambiguous_ref(): tighten up pointer arithmetic

Michael Haggerty <mhagger@alum.mit.edu> writes:
Show 24 quoted lines
> As long as we're being pathologically stingy with mallocs, we might as
> well do the math right and save 6 (!) bytes.
>
> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
> ---
> It is left to the reader to show how another 7 bytes could be saved
> (11 bytes on a 64-bit architecture!)
>
> It probably wouldn't kill performance to use a string_list here
> instead.
>
>  refs.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/refs.c b/refs.c
> index ef9cdea..63b3a71 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -3351,10 +3351,10 @@ char *shorten_unambiguous_ref(const char *refname, int strict)
>  		size_t total_len = 0;
>  		size_t offset = 0;
>  
> -		/* the rule list is NULL terminated, count them first */
> +		/* the rule list is NUL terminated, count them first */

I think this _is_ wrong; it talks about the NULL termination of the ref_rev_parse_rules[] array, not each string that is an element of the array being NUL terminated.

Output from "git grep -e refname_match -e ref_rev_parse_rules" suggests me that we actually could make ref_rev_parse_rules[] a file-scope static to refs.c, remove its NULL termination and convert all the iterators of the array to use ARRAY_SIZE() on it, after dropping the third parameter to refname_match(). That way, we do not have to count them first here.

But that is obviously a separate topic.
Show 7 quoted lines
>  		for (nr_rules = 0; ref_rev_parse_rules[nr_rules]; nr_rules++)
> -			/* no +1 because strlen("%s") < strlen("%.*s") */
> -			total_len += strlen(ref_rev_parse_rules[nr_rules]);
> +			/* -2 for strlen("%.*s") - strlen("%s"); +1 for NUL */
> +			total_len += strlen(ref_rev_parse_rules[nr_rules]) - 2 + 1;
>  
>  		scanf_fmts = xmalloc(nr_rules * sizeof(char *) + total_len);
Michael Haggerty· Jan 10, 2014, 14:16 UTC · re: Junio C Hamano · lore

Re: [PATCH 3/3] shorten_unambiguous_ref(): tighten up pointer arithmetic

On 01/10/2014 12:01 AM, Junio C Hamano wrote:
Show 30 quoted lines
> Michael Haggerty <mhagger@alum.mit.edu> writes:
> 
>> As long as we're being pathologically stingy with mallocs, we might as
>> well do the math right and save 6 (!) bytes.
>>
>> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
>> ---
>> It is left to the reader to show how another 7 bytes could be saved
>> (11 bytes on a 64-bit architecture!)
>>
>> It probably wouldn't kill performance to use a string_list here
>> instead.
>>
>>  refs.c | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/refs.c b/refs.c
>> index ef9cdea..63b3a71 100644
>> --- a/refs.c
>> +++ b/refs.c
>> @@ -3351,10 +3351,10 @@ char *shorten_unambiguous_ref(const char *refname, int strict)
>>  		size_t total_len = 0;
>>  		size_t offset = 0;
>>  
>> -		/* the rule list is NULL terminated, count them first */
>> +		/* the rule list is NUL terminated, count them first */
> 
> I think this _is_ wrong; it talks about the NULL termination of the
> ref_rev_parse_rules[] array, not each string that is an element of
> the array being NUL terminated.

Yes, you're right. Thanks for catching my sloppiness. Would you mind squashing the fix onto my patch?

Show 16 quoted lines
> Output from "git grep -e refname_match -e ref_rev_parse_rules"
> suggests me that we actually could make ref_rev_parse_rules[] a
> file-scope static to refs.c, remove its NULL termination and convert
> all the iterators of the array to use ARRAY_SIZE() on it, after
> dropping the third parameter to refname_match().  That way, we do
> not have to count them first here.
> 
> But that is obviously a separate topic.
> 
>>  		for (nr_rules = 0; ref_rev_parse_rules[nr_rules]; nr_rules++)
>> -			/* no +1 because strlen("%s") < strlen("%.*s") */
>> -			total_len += strlen(ref_rev_parse_rules[nr_rules]);
>> +			/* -2 for strlen("%.*s") - strlen("%s"); +1 for NUL */
>> +			total_len += strlen(ref_rev_parse_rules[nr_rules]) - 2 + 1;
>>  
>>  		scanf_fmts = xmalloc(nr_rules * sizeof(char *) + total_len);

The way the code is written now (e.g., as long as it is not converted to use a string_list or something) needs this loop not only to count the number of rules but also to compute the total_len of the string into which will be written all of the scanf format strings.

As for removing the third argument of refname_match(): although all callers pass it ref_ref_parse_rules, that array is sometimes passed to the function via the alias "ref_fetch_rules". So I suppose somebody wanted to leave the way open to make these two rule sets diverge (though I don't know how likely that is to occur). If we discard the third argument to refname_match(), then we loose the distinction.

Thanks for your feedback, Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/
Junio C Hamano· Jan 10, 2014, 18:03 UTC · re: Michael Haggerty · lore

Re: [PATCH 3/3] shorten_unambiguous_ref(): tighten up pointer arithmetic

Michael Haggerty <mhagger@alum.mit.edu> writes:
Show 5 quoted lines
> As for removing the third argument of refname_match(): although all
> callers pass it ref_ref_parse_rules, that array is sometimes passed to
> the function via the alias "ref_fetch_rules".  So I suppose somebody
> wanted to leave the way open to make these two rule sets diverge (though
> I don't know how likely that is to occur).

It is the other way around. We used to use two separate rules for the normal ref resolution dwimming and dwimming done to decide which remote ref to grab. For example, 'x' in 'git log x' can mean 'refs/remotes/x/HEAD', but 'git fetch origin x' would not grab 'refs/remotes/x/HEAD' at 'origin'. Also, 'git fetch origin v1.0' did not look into 'refs/tags/v1.0' in the 'origin' repository back when these two rules were different.

This was originally done very much on purpose, in order to avoid surprises and to discourage meaningless usage---you would not expect us to be interested in the state of a third repository that was observed by our 'origin' the last time (which we do not even know when).

When we harmonized these two rules later and we #defined out ref_fetch_rules away to avoid potential breakages for in-flight topics. The old synonym can now go safely.

Michael Haggerty· Jan 14, 2014, 03:16 UTC · re: Junio C Hamano · lore

[PATCH] refname_match(): always use the rules in ref_rev_parse_rules

We used to use two separate rules for the normal ref resolution dwimming and dwimming done to decide which remote ref to grab. The third parameter to refname_match() selected which rules to use.

When these two rules were harmonized in
    2011-11-04 dd621df9cd refs DWIMmery: use the same rule for both "git fetch" and others

, ref_fetch_rules was #defined to avoid potential breakages for in-flight topics.

It is now safe to remove the backwards-compatibility code, so remove refname_match()'s third parameter, make ref_rev_parse_rules private to refs.c, and remove ref_fetch_rules entirely.

Suggested-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
See
    http://article.gmane.org/gmane.comp.version-control.git/240305

in which Junio made the suggestion and wrote most of the commit message :-)

 cache.h  | 9 ++++++---
 refs.c   | 6 +++---
 remote.c | 8 ++++----
 3 files changed, 13 insertions(+), 10 deletions(-)
Show changes to 3 files +13 −10

cache.h, refs.c, remote.c

diff --git a/cache.h b/cache.h
index 83a2726..508c49a 100644
--- a/cache.h
+++ b/cache.h
@@ -893,9 +893,12 @@ extern int dwim_log(const char *str, int len, unsigned char *sha1, char **ref);
 extern int interpret_branch_name(const char *str, int len, struct strbuf *);
 extern int get_sha1_mb(const char *str, unsigned char *sha1);
 
-extern int refname_match(const char *abbrev_name, const char *full_name, const char **rules);
-extern const char *ref_rev_parse_rules[];
-#define ref_fetch_rules ref_rev_parse_rules
+/*
+ * Return true iff abbrev_name is a possible abbreviation for
+ * full_name according to the rules defined by ref_rev_parse_rules in
+ * refs.c.
+ */
+extern int refname_match(const char *abbrev_name, const char *full_name);
 
 extern int create_symref(const char *ref, const char *refs_heads_master, const char *logmsg);
 extern int validate_headref(const char *ref);
diff --git a/refs.c b/refs.c
index 490b57b..fc33ee8 100644
--- a/refs.c
+++ b/refs.c
@@ -1880,7 +1880,7 @@ const char *prettify_refname(const char *name)
 		0);
 }
 
-const char *ref_rev_parse_rules[] = {
+static const char *ref_rev_parse_rules[] = {
 	"%.*s",
 	"refs/%.*s",
 	"refs/tags/%.*s",
@@ -1890,12 +1890,12 @@ const char *ref_rev_parse_rules[] = {
 	NULL
 };
 
-int refname_match(const char *abbrev_name, const char *full_name, const char **rules)
+int refname_match(const char *abbrev_name, const char *full_name)
 {
 	const char **p;
 	const int abbrev_name_len = strlen(abbrev_name);
 
-	for (p = rules; *p; p++) {
+	for (p = ref_rev_parse_rules; *p; p++) {
 		if (!strcmp(full_name, mkpath(*p, abbrev_name_len, abbrev_name))) {
 			return 1;
 		}
diff --git a/remote.c b/remote.c
index a89efab..e41251e 100644
--- a/remote.c
+++ b/remote.c
@@ -1000,7 +1000,7 @@ int count_refspec_match(const char *pattern,
 		char *name = refs->name;
 		int namelen = strlen(name);
 
-		if (!refname_match(pattern, name, ref_rev_parse_rules))
+		if (!refname_match(pattern, name))
 			continue;
 
 		/* A match is "weak" if it is with refs outside
@@ -1571,7 +1571,7 @@ int branch_merge_matches(struct branch *branch,
 {
 	if (!branch || i < 0 || i >= branch->merge_nr)
 		return 0;
-	return refname_match(branch->merge[i]->src, refname, ref_fetch_rules);
+	return refname_match(branch->merge[i]->src, refname);
 }
 
 static int ignore_symref_update(const char *refname)
@@ -1624,7 +1624,7 @@ static const struct ref *find_ref_by_name_abbrev(const struct ref *refs, const c
 {
 	const struct ref *ref;
 	for (ref = refs; ref; ref = ref->next) {
-		if (refname_match(name, ref->name, ref_fetch_rules))
+		if (refname_match(name, ref->name))
 			return ref;
 	}
 	return NULL;
@@ -2121,7 +2121,7 @@ static void apply_cas(struct push_cas_option *cas,
 	/* Find an explicit --<option>=<name>[:<value>] entry */
 	for (i = 0; i < cas->nr; i++) {
 		struct push_cas *entry = &cas->entry[i];
-		if (!refname_match(entry->refname, ref->name, ref_rev_parse_rules))
+		if (!refname_match(entry->refname, ref->name))
 			continue;
 		ref->expect_old_sha1 = 1;
 		if (!entry->use_tracking)

-- 
1.8.5.2
Junio C Hamano· Jan 14, 2014, 22:16 UTC · re: Michael Haggerty · lore

Re: [PATCH] refname_match(): always use the rules in ref_rev_parse_rules

Michael Haggerty <mhagger@alum.mit.edu> writes:
Show 24 quoted lines
> We used to use two separate rules for the normal ref resolution
> dwimming and dwimming done to decide which remote ref to grab.  The
> third parameter to refname_match() selected which rules to use.
>
> When these two rules were harmonized in
>
>     2011-11-04 dd621df9cd refs DWIMmery: use the same rule for both "git fetch" and others
>
> , ref_fetch_rules was #defined to avoid potential breakages for
> in-flight topics.
>
> It is now safe to remove the backwards-compatibility code, so remove
> refname_match()'s third parameter, make ref_rev_parse_rules private to
> refs.c, and remove ref_fetch_rules entirely.
>
> Suggested-by: Junio C Hamano <gitster@pobox.com>
> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
> ---
> See
>
>     http://article.gmane.org/gmane.comp.version-control.git/240305
>
> in which Junio made the suggestion and wrote most of the commit
> message :-)
;-) ...and on top of it this may be an obvious endgame follow-up.

was done mindlessly and mechanically, so there may be some slip-ups, though.

 refs.c | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)
Show changes to refs.c +13 −11
diff --git a/refs.c b/refs.c
index 5a10c25..b1c9cf5 100644
--- a/refs.c
+++ b/refs.c
@@ -1886,16 +1886,16 @@ static const char *ref_rev_parse_rules[] = {
 	"refs/tags/%.*s",
 	"refs/heads/%.*s",
 	"refs/remotes/%.*s",
-	"refs/remotes/%.*s/HEAD",
-	NULL
+	"refs/remotes/%.*s/HEAD"
 };
 
 int refname_match(const char *abbrev_name, const char *full_name)
 {
-	const char **p;
+	int i;
 	const int abbrev_name_len = strlen(abbrev_name);
 
-	for (p = ref_rev_parse_rules; *p; p++) {
+	for (i = 0; i < ARRAY_SIZE(ref_rev_parse_rules); i++) {
+		const char **p = &ref_rev_parse_rules[i];
 		if (!strcmp(full_name, mkpath(*p, abbrev_name_len, abbrev_name))) {
 			return 1;
 		}
@@ -1963,11 +1963,13 @@ static char *substitute_branch_name(const char **string, int *len)
 int dwim_ref(const char *str, int len, unsigned char *sha1, char **ref)
 {
 	char *last_branch = substitute_branch_name(&str, &len);
-	const char **p, *r;
+	int i;
+	const char *r;
 	int refs_found = 0;
 
 	*ref = NULL;
-	for (p = ref_rev_parse_rules; *p; p++) {
+	for (i = 0; i < ARRAY_SIZE(ref_rev_parse_rules); i++) {
+		const char **p = &ref_rev_parse_rules[i];
 		char fullref[PATH_MAX];
 		unsigned char sha1_from_ref[20];
 		unsigned char *this_result;
@@ -1994,11 +1996,11 @@ int dwim_ref(const char *str, int len, unsigned char *sha1, char **ref)
 int dwim_log(const char *str, int len, unsigned char *sha1, char **log)
 {
 	char *last_branch = substitute_branch_name(&str, &len);
-	const char **p;
-	int logs_found = 0;
+	int logs_found = 0, i;
 
 	*log = NULL;
-	for (p = ref_rev_parse_rules; *p; p++) {
+	for (i = 0; i < ARRAY_SIZE(ref_rev_parse_rules); i++) {
+		const char **p = &ref_rev_parse_rules[i];
 		struct stat st;
 		unsigned char hash[20];
 		char path[PATH_MAX];
@@ -3368,8 +3370,8 @@ char *shorten_unambiguous_ref(const char *refname, int strict)
 	if (!nr_rules) {
 		size_t total_len = 0;
 
-		/* the rule list is NULL terminated, count them first */
-		for (nr_rules = 0; ref_rev_parse_rules[nr_rules]; nr_rules++)
+		/* Count the bytesize needed to hold rule strings */
+		for (nr_rules = 0; ARRAY_SIZE(ref_rev_parse_rules); nr_rules++)
 			/* no +1 because strlen("%s") < strlen("%.*s") */
 			total_len += strlen(ref_rev_parse_rules[nr_rules]);
Michael Haggerty· Jan 15, 2014, 16:54 UTC · re: Junio C Hamano · lore

Re: [PATCH] refname_match(): always use the rules in ref_rev_parse_rules

On 01/14/2014 11:16 PM, Junio C Hamano wrote:
Show 49 quoted lines
> Michael Haggerty <mhagger@alum.mit.edu> writes:
> 
>> We used to use two separate rules for the normal ref resolution
>> dwimming and dwimming done to decide which remote ref to grab.  The
>> third parameter to refname_match() selected which rules to use.
>>
>> When these two rules were harmonized in
>>
>>     2011-11-04 dd621df9cd refs DWIMmery: use the same rule for both "git fetch" and others
>>
>> , ref_fetch_rules was #defined to avoid potential breakages for
>> in-flight topics.
>>
>> It is now safe to remove the backwards-compatibility code, so remove
>> refname_match()'s third parameter, make ref_rev_parse_rules private to
>> refs.c, and remove ref_fetch_rules entirely.
>>
>> Suggested-by: Junio C Hamano <gitster@pobox.com>
>> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
>> ---
>> See
>>
>>     http://article.gmane.org/gmane.comp.version-control.git/240305
>>
>> in which Junio made the suggestion and wrote most of the commit
>> message :-)
> 
> ;-) ...and on top of it this may be an obvious endgame follow-up.
> 
> was done mindlessly and mechanically, so there may be some slip-ups,
> though.
> 
> 
>  refs.c | 24 +++++++++++++-----------
>  1 file changed, 13 insertions(+), 11 deletions(-)
> 
> diff --git a/refs.c b/refs.c
> index 5a10c25..b1c9cf5 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -1886,16 +1886,16 @@ static const char *ref_rev_parse_rules[] = {
>  	"refs/tags/%.*s",
>  	"refs/heads/%.*s",
>  	"refs/remotes/%.*s",
> -	"refs/remotes/%.*s/HEAD",
> -	NULL
> +	"refs/remotes/%.*s/HEAD"
>  };
> [...rewrite loops to use ARRAY_SIZE()...]

It's doable, but I don't see the point. It's several lines longer (tho that's not significant either). The performance difference should be negligible. There are no users who benefit from knowing the length of the list ahead of time. If we ever decide to make the list vary at runtime your version won't help. Why bother?

Michael
-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/

← back to recent threads