Re: [PATCH 1/3] git-branch: add --track and --no-track options

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Mar 5, 2007, 15:58 UTC
Message-ID: <Pine.LNX.4.63.0703051648330.22628@wbgn013.biozentrum.uni-wuerzburg.de>
In-Reply-To: <45EC3905.7070406@lu.unisi.ch>

Hi,

On Mon, 5 Mar 2007, Paolo Bonzini wrote:

Show 21 quoted lines

> >> +	/* Try with a wildcard match now.  */
> >> +	sscanf(value, "refs/%*[^/]/*:%nrefs/remotes/%*[^/]/*%n",
> >> +	       &len_first, &len_second);
> >> +	if (len_first != -1 && len_second != -1
> >> +	    && (len_second - 2) - len_first == remote_len + 13
> >> +	    && !strncmp(value + len_first, start_ref, remote_len + 13)) {
> >> +		/* Replace the star with the remote branch name.  */
> >> +		asprintf(&config_repo, "%.*s%s",
> >> +			 len_first - 3, value,
> >> +			 start_ref + remote_len + 13);
> > 
> > Same here:
> > 
> > 	else if (!prefixcmp(value, "refs/") && (p = strchr(value, ':')) &&
> > 			!memcmp(p + 1, start_ref, remote_len) &&
> > 			!strcmp(p + 1 + remote_len, "/*")) {
> > 		config_repo = xstrdup(value);
> > 		config_repo[p - value] = '\0';
> > 	}
> 
> This is not what my code does.

You are completely correct. I overlooked the need to check for "/*" in the first part. So, this should go before the memcmp() line:

			!prefixcmp(p - 2, "/*") &&

I also forgot to replace "*" by the branch name. The if block should look like this, then:

		config_repo = xmalloc(p - value + start_len - remote_len);
		strncpy(config_repo, value, p - 1 - value);
		strcat(config_repo, start_ref + remote_len);

As usual, completely untested.

Show 5 quoted lines

> Are you saying that yours is correct, or that it should actually be the 
> same in practice, or just that I should avoid sscanf/asprintf (which I 
> won't do, since I got mildly insane after writing half of the function 
> without sscanf -- and then decided that the C library is there to be 
> used).

Well, you make a good case there. I am only mildly concerned that this might not work on some obscure platforms (including Windows and SunOS), and that we are not even realizing that because you do not check the return value of sscanf().

Ciao, Dscho