# [PATCH 4/5] Keep '*' in pattern refspecs 4 messages from 2009-03-06 to 2009-03-07. Participants: Daniel Barkalow, Junio C Hamano. Thread: https://gitlist.dev/t/18166 ## Daniel Barkalow, 2009-03-06 04:56 Subject: [PATCH 4/5] Keep '*' in pattern refspecs Message-ID: URL: https://gitlist.dev/e/alpine.LNX.1.00.0903052355480.19665%40iabervon.org ``` In order to do anything more capable with refspecs, the first step is to keep the entire input. Additionally, validate patterns by checking for the ref matching the rules for a pattern as given by check_ref_format(). This requires a slight change to make it require the '*' to be at the beginning of a path component. Signed-off-by: Daniel Barkalow --- refs.c | 4 +--- remote.c | 24 +++++++++--------------- 2 files changed, 10 insertions(+), 18 deletions(-) diff --git a/refs.c b/refs.c index 6eb5f53..a50ba79 100644 --- a/refs.c +++ b/refs.c @@ -718,9 +718,7 @@ int check_ref_format(const char *ref) while ((ch = *cp++) != 0) { bad_type = bad_ref_char(ch); if (bad_type) { - return (bad_type == 2 && !*cp) - ? CHECK_REF_FORMAT_WILDCARD - : CHECK_REF_FORMAT_ERROR; + return CHECK_REF_FORMAT_ERROR; } if (ch == '/') break; diff --git a/remote.c b/remote.c index 93fd03d..d0ce4c6 100644 --- a/remote.c +++ b/remote.c @@ -10,8 +10,8 @@ static struct refspec s_tag_refspec = { 0, 1, 0, - "refs/tags/", - "refs/tags/" + "refs/tags/*", + "refs/tags/*" }; const struct refspec *tag_refspec = &s_tag_refspec; @@ -451,16 +451,11 @@ static void read_config(void) */ static int verify_refname(char *name, int is_glob) { - int result, len = -1; + int result; - if (is_glob) { - len = strlen(name); - assert(name[len - 1] == '/'); - name[len - 1] = '\0'; - } result = check_ref_format(name); - if (is_glob) - name[len - 1] = '/'; + if (is_glob && result == CHECK_REF_FORMAT_WILDCARD) + result = CHECK_REF_FORMAT_OK; return result; } @@ -517,7 +512,7 @@ static struct refspec *parse_refspec_internal(int nr_refspec, const char **refsp if (rhs) { size_t rlen = strlen(++rhs); is_glob = (2 <= rlen && !strcmp(rhs + rlen - 2, "/*")); - rs[i].dst = xstrndup(rhs, rlen - is_glob); + rs[i].dst = xstrndup(rhs, rlen); } llen = (rhs ? (rhs - lhs - 1) : strlen(lhs)); @@ -525,7 +520,6 @@ static struct refspec *parse_refspec_internal(int nr_refspec, const char **refsp if ((rhs && !is_glob) || (!rhs && fetch)) goto invalid; is_glob = 1; - llen--; } else if (rhs && is_glob) { goto invalid; } @@ -722,10 +716,10 @@ int remote_has_url(struct remote *remote, const char *url) static int name_fits_pattern(const char *key, const char *name, const char *value, char **result) { - size_t klen = strlen(key); - int ret = !strncmp(key, name, klen); + size_t klen = strchr(key, '*') - key; + int ret = !strncmp(name, key, klen); if (ret && value) { - size_t vlen = strlen(value); + size_t vlen = strchr(value, '*') - value; *result = xmalloc(vlen + strlen(name) - klen + 1); -- 1.6.1.286.gd33a4.dirty ``` ## Junio C Hamano, 2009-03-06 08:18 Subject: Re: [PATCH 4/5] Keep '*' in pattern refspecs Message-ID: <7vsklrt66z.fsf@gitster.siamese.dyndns.org> URL: https://gitlist.dev/e/7vsklrt66z.fsf%40gitster.siamese.dyndns.org In-Reply-To: ``` Daniel Barkalow writes: > In order to do anything more capable with refspecs, the first step is > to keep the entire input. Additionally, validate patterns by checking > for the ref matching the rules for a pattern as given by > check_ref_format(). This requires a slight change to make it require > the '*' to be at the beginning of a path component. I had a brief "Huh?" moment wondering about this "slight change", but at this stage it does not change the rule at all ("/*" still must happen at the end of the string), so there actually is no change. > diff --git a/remote.c b/remote.c > index 93fd03d..d0ce4c6 100644 > --- a/remote.c > +++ b/remote.c > @@ -722,10 +716,10 @@ int remote_has_url(struct remote *remote, const char *url) > static int name_fits_pattern(const char *key, const char *name, > const char *value, char **result) > { > - size_t klen = strlen(key); > - int ret = !strncmp(key, name, klen); > + size_t klen = strchr(key, '*') - key; > + int ret = !strncmp(name, key, klen); Any particular reason why the first parameters to strncmp() were swapped? > if (ret && value) { > - size_t vlen = strlen(value); > + size_t vlen = strchr(value, '*') - value; We would want protection from programming error here, to catch keys and values without any asterisk. This comment also applies to [5/5]. > *result = xmalloc(vlen + > strlen(name) - > klen + 1); ``` ## Daniel Barkalow, 2009-03-06 17:21 Subject: Re: [PATCH 4/5] Keep '*' in pattern refspecs Message-ID: URL: https://gitlist.dev/e/alpine.LNX.1.00.0903061157540.19665%40iabervon.org In-Reply-To: <7vsklrt66z.fsf@gitster.siamese.dyndns.org> ``` On Fri, 6 Mar 2009, Junio C Hamano wrote: > Daniel Barkalow writes: > > > In order to do anything more capable with refspecs, the first step is > > to keep the entire input. Additionally, validate patterns by checking > > for the ref matching the rules for a pattern as given by > > check_ref_format(). This requires a slight change to make it require > > the '*' to be at the beginning of a path component. > > I had a brief "Huh?" moment wondering about this "slight change", but at > this stage it does not change the rule at all ("/*" still must happen at > the end of the string), so there actually is no change. Ah, yes. A slight change to the code in refs.c to enforce the rule that was enforced previously in remote.c. > > diff --git a/remote.c b/remote.c > > index 93fd03d..d0ce4c6 100644 > > --- a/remote.c > > +++ b/remote.c > > @@ -722,10 +716,10 @@ int remote_has_url(struct remote *remote, const char *url) > > static int name_fits_pattern(const char *key, const char *name, > > const char *value, char **result) > > { > > - size_t klen = strlen(key); > > - int ret = !strncmp(key, name, klen); > > + size_t klen = strchr(key, '*') - key; > > + int ret = !strncmp(name, key, klen); > > Any particular reason why the first parameters to strncmp() were swapped? An artifact of how I cleaned up the series that I didn't notice; I originally wrote it from scratch with strncmp, with an arbitrary argument order. Then I split out 2/5 using prefixcmp, and 3/5 keeping prefixcmp's order, and didn't notice I didn't need the line of diff. So not really. > > if (ret && value) { > > - size_t vlen = strlen(value); > > + size_t vlen = strchr(value, '*') - value; > > We would want protection from programming error here, to catch keys and > values without any asterisk. This comment also applies to [5/5]. Good idea. > > *result = xmalloc(vlen + > > strlen(name) - > > klen + 1); > -Daniel *This .sig left intentionally blank* ``` ## Daniel Barkalow, 2009-03-07 06:11 Subject: [PATCH v2 4/5] Keep '*' in pattern refspecs Message-ID: URL: https://gitlist.dev/e/alpine.LNX.1.00.0903070106470.19665%40iabervon.org In-Reply-To: ``` In order to do anything more capable with refspecs, the first step is to keep the entire input. Additionally, validate patterns by checking for the ref matching the rules for a pattern as given by check_ref_format(). This requires a slight change to check_ref_format() to make it enforce the requirement that the '*' immediately follow a '/'. Signed-off-by: Daniel Barkalow --- Don't arbitrarily swap argument order; check for missing '*' in pattern sides. refs.c | 4 +--- remote.c | 35 +++++++++++++++++++---------------- 2 files changed, 20 insertions(+), 19 deletions(-) diff --git a/refs.c b/refs.c index 6eb5f53..a50ba79 100644 --- a/refs.c +++ b/refs.c @@ -718,9 +718,7 @@ int check_ref_format(const char *ref) while ((ch = *cp++) != 0) { bad_type = bad_ref_char(ch); if (bad_type) { - return (bad_type == 2 && !*cp) - ? CHECK_REF_FORMAT_WILDCARD - : CHECK_REF_FORMAT_ERROR; + return CHECK_REF_FORMAT_ERROR; } if (ch == '/') break; diff --git a/remote.c b/remote.c index 5638766..d596a48 100644 --- a/remote.c +++ b/remote.c @@ -10,8 +10,8 @@ static struct refspec s_tag_refspec = { 0, 1, 0, - "refs/tags/", - "refs/tags/" + "refs/tags/*", + "refs/tags/*" }; const struct refspec *tag_refspec = &s_tag_refspec; @@ -451,16 +451,11 @@ static void read_config(void) */ static int verify_refname(char *name, int is_glob) { - int result, len = -1; + int result; - if (is_glob) { - len = strlen(name); - assert(name[len - 1] == '/'); - name[len - 1] = '\0'; - } result = check_ref_format(name); - if (is_glob) - name[len - 1] = '/'; + if (is_glob && result == CHECK_REF_FORMAT_WILDCARD) + result = CHECK_REF_FORMAT_OK; return result; } @@ -517,7 +512,7 @@ static struct refspec *parse_refspec_internal(int nr_refspec, const char **refsp if (rhs) { size_t rlen = strlen(++rhs); is_glob = (2 <= rlen && !strcmp(rhs + rlen - 2, "/*")); - rs[i].dst = xstrndup(rhs, rlen - is_glob); + rs[i].dst = xstrndup(rhs, rlen); } llen = (rhs ? (rhs - lhs - 1) : strlen(lhs)); @@ -525,7 +520,6 @@ static struct refspec *parse_refspec_internal(int nr_refspec, const char **refsp if ((rhs && !is_glob) || (!rhs && fetch)) goto invalid; is_glob = 1; - llen--; } else if (rhs && is_glob) { goto invalid; } @@ -720,12 +714,21 @@ int remote_has_url(struct remote *remote, const char *url) } static int match_name_with_pattern(const char *key, const char *name, const char *value, char **result) { - size_t klen = strlen(key); - int ret = !strncmp(key, name, klen); + const char *kstar = strchr(key, '*'); + size_t klen; + int ret; + if (!kstar) + die("Key '%s' of pattern had no '*'", key); + klen = kstar - key; + ret = !strncmp(key, name, klen); if (ret && value) { - size_t vlen = strlen(value); + const char *vstar = strchr(value, '*'); + size_t vlen; + if (!vstar) + die("Value '%s' of pattern has no '*'", value); + vlen = vstar - value; *result = xmalloc(vlen + strlen(name) - klen + 1); -- 1.6.1.286.gd33a4.dirty ```