threads / patch / 18166

patch, 5 partsKeep '*' in pattern refspecs

Subject: [PATCH 4/5] Keep '*' in pattern refspecs

## tl;dr

4 messages between Mar 6, 2009 and Mar 7, 2009. Diffs are folded; open one to read it.

replies: 3people: 2as markdown or json

Daniel Barkalow· Mar 6, 2009, 04:56 UTC · lore

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 <barkalow@iabervon.org>
---
 refs.c   |    4 +---
 remote.c |   24 +++++++++---------------
 2 files changed, 10 insertions(+), 18 deletions(-)

Show changes to 2 files +10 −18

refs.c, remote.c

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· Mar 6, 2009, 08:18 UTC · re: Daniel Barkalow · lore

Re: [PATCH 4/5] Keep '*' in pattern refspecs

Daniel Barkalow <barkalow@iabervon.org> writes:

Show 5 quoted lines

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

Show 12 quoted lines

> 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· Mar 6, 2009, 17:21 UTC · re: Junio C Hamano · lore

Re: [PATCH 4/5] Keep '*' in pattern refspecs

On Fri, 6 Mar 2009, Junio C Hamano wrote:

Show 11 quoted lines

> Daniel Barkalow <barkalow@iabervon.org> 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.

Show 14 quoted lines

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

Show 6 quoted lines

> >  	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· Mar 7, 2009, 06:11 UTC · re: Daniel Barkalow · lore

[PATCH v2 4/5] Keep '*' in pattern refspecs

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 <barkalow@iabervon.org>
---
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(-)

Show changes to 2 files +19 −18

refs.c, remote.c

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