From: Jeff King <peff@peff.net>
Date: Thu, 14 Jul 2016 21:35:35 GMT
Subject: Re: [PATCHv6 2/4] receive-pack: implement advertising and receiving push options
Message-ID: <20160714213534.GB20645@sigill.intra.peff.net>
In-Reply-To: <20160714212454.16643-1-sbeller@google.com>

On Thu, Jul 14, 2016 at 02:24:54PM -0700, Stefan Beller wrote:

> # interdiff to v5:
> [...giant deletion...]

Much nicer. :)

> +static struct string_list *read_push_options(void)
> +{
> +	struct string_list *ret = xmalloc(sizeof(*ret));

This struck me as a little non-idiomatic for our code base. The usual
technique is to take a pointer to a stack-allocated struct, and write
into that.

I guess that here:

> @@ -1774,6 +1806,9 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix)
>  		const char *unpack_status = NULL;
>  		struct string_list *push_options = NULL;
>  
> +		if (use_push_options)
> +			push_options = read_push_options();
> +

You will want to later check whether push_options is NULL. But you can
also just check push_options.nr.

-Peff