Re: [PATCHv2] unpack-trees: move checkout state into check_updates

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Stefan Beller <sbeller@xxxxxxxxxx> writes:

> The checkout state was introduced via 16da134b1f9
> (read-trees: refactor the unpack_trees() part, 2006-07-30). An attempt to
> refactor the checkout state was done in b56aa5b268e (unpack-trees: pass
> checkout state explicitly to check_updates(), 2016-09-13), but we can
> go even further.
>
> The `struct checkout state` is not used in unpack_trees apart from
> initializing it, so move it into the function that makes use of it,
> which is `check_updates`.
>
> Signed-off-by: Stefan Beller <sbeller@xxxxxxxxxx>
> ---

I'd add René's Reviewed-by: here.

> -static int check_updates(struct unpack_trees_options *o,
> -			 const struct checkout *state)
> +static int check_updates(struct unpack_trees_options *o)
>  {
>  	unsigned cnt = 0, total = 0;
>  	struct progress *progress = NULL;
>  	struct index_state *index = &o->result;
> -	int i;
> -	int errs = 0;
> +	struct checkout state = CHECKOUT_INIT;
> +	int i, errs = 0;
> +
> +	state.force = 1;
> +	state.quiet = 1;
> +	state.refresh_cache = 1;
> +	state.istate = index;

I think moving heavier and initialized variables earlier and more
lightweight and ephemeral ones like "i" later does make it easier to
follow.  "errs" has the significance and the lifetime similar to
cnt/total, and logically should be higher, though.  It is not a big
enough deal to reroll (but as your futzing of the variable definition
order was not a big enough deal to do in this patch, either, so...).

Queued.  Thanks.




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]