Re: [PATCH 33/67] read_branches_file: replace strcpy with xstrdup

From: Jeff King <peff@peff.net>
Date: Sep 17, 2015, 11:32 UTC
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:

Show 21 quoted lines

>  	/*
>  	 * 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