{"thread":{"id":"18166","subject":"[PATCH 4/5] Keep '*' in pattern refspecs","startedAt":"2009-03-06T04:56:22Z","lastAt":"2009-03-07T06:11:36Z","messageCount":4,"participants":["Daniel Barkalow","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"107144","messageId":"alpine.LNX.1.00.0903052355480.19665@iabervon.org","threadId":"18166","inReplyTo":null,"subject":"[PATCH 4/5] Keep '*' in pattern refspecs","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2009-03-06T04:56:22Z","receivedAt":"2009-03-06T04:56:22Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"In order to do anything more capable with refspecs, the first step is\nto keep the entire input. Additionally, validate patterns by checking\nfor the ref matching the rules for a pattern as given by\ncheck_ref_format(). This requires a slight change to make it require\nthe '*' to be at the beginning of a path component.\n\nSigned-off-by: Daniel Barkalow <barkalow@iabervon.org>\n---\n refs.c   |    4 +---\n remote.c |   24 +++++++++---------------\n 2 files changed, 10 insertions(+), 18 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 6eb5f53..a50ba79 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -718,9 +718,7 @@ int check_ref_format(const char *ref)\n \t\twhile ((ch = *cp++) != 0) {\n \t\t\tbad_type = bad_ref_char(ch);\n \t\t\tif (bad_type) {\n-\t\t\t\treturn (bad_type == 2 && !*cp)\n-\t\t\t\t\t? CHECK_REF_FORMAT_WILDCARD\n-\t\t\t\t\t: CHECK_REF_FORMAT_ERROR;\n+\t\t\t\treturn CHECK_REF_FORMAT_ERROR;\n \t\t\t}\n \t\t\tif (ch == '/')\n \t\t\t\tbreak;\ndiff --git a/remote.c b/remote.c\nindex 93fd03d..d0ce4c6 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -10,8 +10,8 @@ static struct refspec s_tag_refspec = {\n \t0,\n \t1,\n \t0,\n-\t\"refs/tags/\",\n-\t\"refs/tags/\"\n+\t\"refs/tags/*\",\n+\t\"refs/tags/*\"\n };\n \n const struct refspec *tag_refspec = &s_tag_refspec;\n@@ -451,16 +451,11 @@ static void read_config(void)\n  */\n static int verify_refname(char *name, int is_glob)\n {\n-\tint result, len = -1;\n+\tint result;\n \n-\tif (is_glob) {\n-\t\tlen = strlen(name);\n-\t\tassert(name[len - 1] == '/');\n-\t\tname[len - 1] = '\\0';\n-\t}\n \tresult = check_ref_format(name);\n-\tif (is_glob)\n-\t\tname[len - 1] = '/';\n+\tif (is_glob && result == CHECK_REF_FORMAT_WILDCARD)\n+\t\tresult = CHECK_REF_FORMAT_OK;\n \treturn result;\n }\n \n@@ -517,7 +512,7 @@ static struct refspec *parse_refspec_internal(int nr_refspec, const char **refsp\n \t\tif (rhs) {\n \t\t\tsize_t rlen = strlen(++rhs);\n \t\t\tis_glob = (2 <= rlen && !strcmp(rhs + rlen - 2, \"/*\"));\n-\t\t\trs[i].dst = xstrndup(rhs, rlen - is_glob);\n+\t\t\trs[i].dst = xstrndup(rhs, rlen);\n \t\t}\n \n \t\tllen = (rhs ? (rhs - lhs - 1) : strlen(lhs));\n@@ -525,7 +520,6 @@ static struct refspec *parse_refspec_internal(int nr_refspec, const char **refsp\n \t\t\tif ((rhs && !is_glob) || (!rhs && fetch))\n \t\t\t\tgoto invalid;\n \t\t\tis_glob = 1;\n-\t\t\tllen--;\n \t\t} else if (rhs && is_glob) {\n \t\t\tgoto invalid;\n \t\t}\n@@ -722,10 +716,10 @@ int remote_has_url(struct remote *remote, const char *url)\n static int name_fits_pattern(const char *key, const char *name,\n \t\t\t     const char *value, char **result)\n {\n-\tsize_t klen = strlen(key);\n-\tint ret = !strncmp(key, name, klen);\n+\tsize_t klen = strchr(key, '*') - key;\n+\tint ret = !strncmp(name, key, klen);\n \tif (ret && value) {\n-\t\tsize_t vlen = strlen(value);\n+\t\tsize_t vlen = strchr(value, '*') - value;\n \t\t*result = xmalloc(vlen +\n \t\t\t\t  strlen(name) -\n \t\t\t\t  klen + 1);\n-- \n1.6.1.286.gd33a4.dirty\n"},{"id":"107164","messageId":"7vsklrt66z.fsf@gitster.siamese.dyndns.org","threadId":"18166","inReplyTo":"alpine.LNX.1.00.0903052355480.19665@iabervon.org","subject":"Re: [PATCH 4/5] Keep '*' in pattern refspecs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-03-06T08:18:44Z","receivedAt":"2009-03-06T08:18:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Daniel Barkalow <barkalow@iabervon.org> writes:\n\n> In order to do anything more capable with refspecs, the first step is\n> to keep the entire input. Additionally, validate patterns by checking\n> for the ref matching the rules for a pattern as given by\n> check_ref_format(). This requires a slight change to make it require\n> the '*' to be at the beginning of a path component.\n\nI had a brief \"Huh?\" moment wondering about this \"slight change\", but at\nthis stage it does not change the rule at all (\"/*\" still must happen at\nthe end of the string), so there actually is no change.\n\n> diff --git a/remote.c b/remote.c\n> index 93fd03d..d0ce4c6 100644\n> --- a/remote.c\n> +++ b/remote.c\n> @@ -722,10 +716,10 @@ int remote_has_url(struct remote *remote, const char *url)\n>  static int name_fits_pattern(const char *key, const char *name,\n>  \t\t\t     const char *value, char **result)\n>  {\n> -\tsize_t klen = strlen(key);\n> -\tint ret = !strncmp(key, name, klen);\n> +\tsize_t klen = strchr(key, '*') - key;\n> +\tint ret = !strncmp(name, key, klen);\n\nAny particular reason why the first parameters to strncmp() were swapped?\n\n>  \tif (ret && value) {\n> -\t\tsize_t vlen = strlen(value);\n> +\t\tsize_t vlen = strchr(value, '*') - value;\n\nWe would want protection from programming error here, to catch keys and\nvalues without any asterisk.  This comment also applies to [5/5].\n\n>  \t\t*result = xmalloc(vlen +\n>  \t\t\t\t  strlen(name) -\n>  \t\t\t\t  klen + 1);\n"},{"id":"107233","messageId":"alpine.LNX.1.00.0903061157540.19665@iabervon.org","threadId":"18166","inReplyTo":"7vsklrt66z.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 4/5] Keep '*' in pattern refspecs","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2009-03-06T17:21:47Z","receivedAt":"2009-03-06T17:21:47Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Fri, 6 Mar 2009, Junio C Hamano wrote:\n\n> Daniel Barkalow <barkalow@iabervon.org> writes:\n> \n> > In order to do anything more capable with refspecs, the first step is\n> > to keep the entire input. Additionally, validate patterns by checking\n> > for the ref matching the rules for a pattern as given by\n> > check_ref_format(). This requires a slight change to make it require\n> > the '*' to be at the beginning of a path component.\n> \n> I had a brief \"Huh?\" moment wondering about this \"slight change\", but at\n> this stage it does not change the rule at all (\"/*\" still must happen at\n> the end of the string), so there actually is no change.\n\nAh, yes. A slight change to the code in refs.c to enforce the rule that \nwas enforced previously in remote.c.\n\n> > diff --git a/remote.c b/remote.c\n> > index 93fd03d..d0ce4c6 100644\n> > --- a/remote.c\n> > +++ b/remote.c\n> > @@ -722,10 +716,10 @@ int remote_has_url(struct remote *remote, const char *url)\n> >  static int name_fits_pattern(const char *key, const char *name,\n> >  \t\t\t     const char *value, char **result)\n> >  {\n> > -\tsize_t klen = strlen(key);\n> > -\tint ret = !strncmp(key, name, klen);\n> > +\tsize_t klen = strchr(key, '*') - key;\n> > +\tint ret = !strncmp(name, key, klen);\n> \n> Any particular reason why the first parameters to strncmp() were swapped?\n\nAn artifact of how I cleaned up the series that I didn't notice; I \noriginally wrote it from scratch with strncmp, with an arbitrary argument \norder. Then I split out 2/5 using prefixcmp, and 3/5 keeping prefixcmp's \norder, and didn't notice I didn't need the line of diff. So not really.\n\n> >  \tif (ret && value) {\n> > -\t\tsize_t vlen = strlen(value);\n> > +\t\tsize_t vlen = strchr(value, '*') - value;\n> \n> We would want protection from programming error here, to catch keys and\n> values without any asterisk.  This comment also applies to [5/5].\n\nGood idea.\n\n> >  \t\t*result = xmalloc(vlen +\n> >  \t\t\t\t  strlen(name) -\n> >  \t\t\t\t  klen + 1);\n> \n\n\t-Daniel\n*This .sig left intentionally blank*\n"},{"id":"107276","messageId":"alpine.LNX.1.00.0903070106470.19665@iabervon.org","threadId":"18166","inReplyTo":"alpine.LNX.1.00.0903052355480.19665@iabervon.org","subject":"[PATCH v2 4/5] Keep '*' in pattern refspecs","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2009-03-07T06:11:36Z","receivedAt":"2009-03-07T06:11:36Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"In order to do anything more capable with refspecs, the first step is\nto keep the entire input. Additionally, validate patterns by checking\nfor the ref matching the rules for a pattern as given by\ncheck_ref_format(). This requires a slight change to\ncheck_ref_format() to make it enforce the requirement that the '*'\nimmediately follow a '/'.\n\nSigned-off-by: Daniel Barkalow <barkalow@iabervon.org>\n---\nDon't arbitrarily swap argument order; check for missing '*' in pattern \nsides.\n\n refs.c   |    4 +---\n remote.c |   35 +++++++++++++++++++----------------\n 2 files changed, 20 insertions(+), 19 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 6eb5f53..a50ba79 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -718,9 +718,7 @@ int check_ref_format(const char *ref)\n \t\twhile ((ch = *cp++) != 0) {\n \t\t\tbad_type = bad_ref_char(ch);\n \t\t\tif (bad_type) {\n-\t\t\t\treturn (bad_type == 2 && !*cp)\n-\t\t\t\t\t? CHECK_REF_FORMAT_WILDCARD\n-\t\t\t\t\t: CHECK_REF_FORMAT_ERROR;\n+\t\t\t\treturn CHECK_REF_FORMAT_ERROR;\n \t\t\t}\n \t\t\tif (ch == '/')\n \t\t\t\tbreak;\ndiff --git a/remote.c b/remote.c\nindex 5638766..d596a48 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -10,8 +10,8 @@ static struct refspec s_tag_refspec = {\n \t0,\n \t1,\n \t0,\n-\t\"refs/tags/\",\n-\t\"refs/tags/\"\n+\t\"refs/tags/*\",\n+\t\"refs/tags/*\"\n };\n \n const struct refspec *tag_refspec = &s_tag_refspec;\n@@ -451,16 +451,11 @@ static void read_config(void)\n  */\n static int verify_refname(char *name, int is_glob)\n {\n-\tint result, len = -1;\n+\tint result;\n \n-\tif (is_glob) {\n-\t\tlen = strlen(name);\n-\t\tassert(name[len - 1] == '/');\n-\t\tname[len - 1] = '\\0';\n-\t}\n \tresult = check_ref_format(name);\n-\tif (is_glob)\n-\t\tname[len - 1] = '/';\n+\tif (is_glob && result == CHECK_REF_FORMAT_WILDCARD)\n+\t\tresult = CHECK_REF_FORMAT_OK;\n \treturn result;\n }\n \n@@ -517,7 +512,7 @@ static struct refspec *parse_refspec_internal(int nr_refspec, const char **refsp\n \t\tif (rhs) {\n \t\t\tsize_t rlen = strlen(++rhs);\n \t\t\tis_glob = (2 <= rlen && !strcmp(rhs + rlen - 2, \"/*\"));\n-\t\t\trs[i].dst = xstrndup(rhs, rlen - is_glob);\n+\t\t\trs[i].dst = xstrndup(rhs, rlen);\n \t\t}\n \n \t\tllen = (rhs ? (rhs - lhs - 1) : strlen(lhs));\n@@ -525,7 +520,6 @@ static struct refspec *parse_refspec_internal(int nr_refspec, const char **refsp\n \t\t\tif ((rhs && !is_glob) || (!rhs && fetch))\n \t\t\t\tgoto invalid;\n \t\t\tis_glob = 1;\n-\t\t\tllen--;\n \t\t} else if (rhs && is_glob) {\n \t\t\tgoto invalid;\n \t\t}\n@@ -720,12 +714,21 @@ int remote_has_url(struct remote *remote, const char *url)\n }\n \n static int match_name_with_pattern(const char *key, const char *name,\n \t\t\t\t   const char *value, char **result)\n {\n-\tsize_t klen = strlen(key);\n-\tint ret = !strncmp(key, name, klen);\n+\tconst char *kstar = strchr(key, '*');\n+\tsize_t klen;\n+\tint ret;\n+\tif (!kstar)\n+\t\tdie(\"Key '%s' of pattern had no '*'\", key);\n+\tklen = kstar - key;\n+\tret = !strncmp(key, name, klen);\n \tif (ret && value) {\n-\t\tsize_t vlen = strlen(value);\n+\t\tconst char *vstar = strchr(value, '*');\n+\t\tsize_t vlen;\n+\t\tif (!vstar)\n+\t\t\tdie(\"Value '%s' of pattern has no '*'\", value);\n+\t\tvlen = vstar - value;\n \t\t*result = xmalloc(vlen +\n \t\t\t\t  strlen(name) -\n \t\t\t\t  klen + 1);\n-- \n1.6.1.286.gd33a4.dirty\n"}]}