Re: [PATCHv5 1/2] clone: respect additional configured fetch refspecs during initial fetch

From: Jeff King <peff@peff.net>
Date: Jun 17, 2017, 11:22 UTC
Message-ID: <20170617112228.vugswym4o4owf6wj@sigill.intra.peff.net>
In-Reply-To: <20170616173849.8071-2-szeder.dev@gmail.com>

On Fri, Jun 16, 2017 at 07:38:48PM +0200, SZEDER Gábor wrote:

Show 12 quoted lines

> +void add_and_parse_fetch_refspec(struct remote *remote, const char *refspec)
> +{
> +	struct refspec *rs;
> +
> +	add_fetch_refspec(remote, refspec);
> +	rs = parse_fetch_refspec(1, &refspec);
> +	REALLOC_ARRAY(remote->fetch, remote->fetch_refspec_nr);
> +	remote->fetch[remote->fetch_refspec_nr - 1] = *rs;
> +
> +	/* Not free_refspecs(), as we copied its pointers above */
> +	free(rs);
> +}

I'd still prefer this to have:

  if (!remote->fetch && remote->fetch_refspec_nr)
	BUG("attempt to add refspec to uninitialized list");

at the top, as otherwise this case writes garbage into remote->fetch[0].

I see you have another series dealing with the lazy parsing, but I haven't looked at it yet (hopefully this danger would just go away after that).

Other than that, the patch looks fine to me.

-Peff