Hi,
On Mon, 5 Mar 2007, Paolo Bonzini wrote:
> >> + /* 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.
> 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
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html