Re: [PATCH 02/12] upload-pack: move static vars to upload_pack_data

From: Jeff King <peff@peff.net>
Date: May 27, 2020, 18:06 UTC
Message-ID: <20200527180611.GA4020820@coredump.intra.peff.net>
In-Reply-To: <20200527164742.23067-3-chriscool@tuxfamily.org>

On Wed, May 27, 2020 at 06:47:32PM +0200, Christian Couder wrote:

> As we cleanup 'upload-pack.c' by using 'struct upload_pack_data'
> more thoroughly, let's move the 'no_done', 'daemon_mode' and
> 'timeout' variables into this struct.

Makes sense.

> They are only used by protocol v0 code since protocol v2 assumes
> certain baseline capabilities, but rolling them into
> upload_pack_data and just letting v2 code ignore them as it does
> now is more coherent and cleaner.

Is it perhaps worth keeping these v0-only fields grouped together within the struct, along with a comment? That way nobody has to repeat your research later into which ones are used and which ones are not.

> -static unsigned int timeout;

This was unsigned, but gets moved as...

Show 5 quoted lines

> @@ -83,18 +80,21 @@ struct upload_pack_data {
>  	timestamp_t deepen_since;
>  	int deepen_rev_list;
>  	int deepen_relative;
> +	int timeout;

...a signed int.

It probably doesn't matter much in practice for the values we'd give it, but the alarm() to which it's ultimately fed takes an unsigned int.

-Peff