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

2014-01-15 Thread Michael Haggerty
On 01/14/2014 11:16 PM, Junio C Hamano wrote:
> Michael Haggerty  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 
>> Signed-off-by: Michael Haggerty 
>> ---
>> 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
mhag...@alum.mit.edu
http://softwareswirl.blogspot.com/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2014-01-14 Thread Junio C Hamano
Michael Haggerty  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 
> Signed-off-by: Michael Haggerty 
> ---
> 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"
 };
 
 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]);
 
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html