From: Jeff King <peff@peff.net>
Date: Thu, 17 Sep 2015 11:32:31 GMT
Subject: Re: [PATCH 33/67] read_branches_file: replace strcpy with xstrdup
Message-ID: <20150917113231.GB22085@sigill.intra.peff.net>
In-Reply-To: <20150917112856.GA22085@sigill.intra.peff.net>

On Thu, Sep 17, 2015 at 07:28:56AM -0400, Jeff King wrote:

>  	/*
>  	 * The branches file would have URL and optionally
>  	 * #branch specified.  The "master" (or specified) branch is
> -	 * fetched and stored in the local branch of the same name.
> +	 * fetched and stored in the local branch matching the
> +	 * remote name.
>  	 */
> -	frag = strchr(p, '#');
> -	if (frag) {
> +	frag = strchr(buf.buf, '#');
> +	if (frag)
>  		*(frag++) = '\0';
> -		strbuf_addf(&branch, "refs/heads/%s", frag);
> -	} else
> -		strbuf_addstr(&branch, "refs/heads/master");
> +	else
> +		frag = "master";
> +
> +	add_url_alias(remote, strbuf_detach(&buf, NULL));
> +	add_fetch_refspec(remote, xstrfmt("refs/heads/%s:refs/heads/%s",
> +					  frag, remote->name));

There is a little bit of subtlety here. "frag" points into the strbuf.
We then detach the strbuf, and assume that "frag" is still valid. This
works fine, as we retain the same buffer when detaching. But I wonder if
it is violating the strbuf abstraction too much.

We can xstrdup() the frag section, but it just adds a little more
tedious book-keeping to the function.

-Peff