Re: [PATCH 39/83] builtin/apply: move 'ws_error_action' into 'struct apply_state'

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 26, 2016, 20:36 UTC
Message-ID: <xmqqtwiods5n.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <1461504863-15946-40-git-send-email-chriscool@tuxfamily.org>

Christian Couder <christian.couder@gmail.com> writes:

Show 29 quoted lines

> +enum ws_error_action {
> +	nowarn_ws_error,
> +	warn_on_ws_error,
> +	die_on_ws_error,
> +	correct_ws_error
> +};
> +
>  struct apply_state {
>  	const char *prefix;
>  	int prefix_length;
> @@ -80,6 +87,8 @@ struct apply_state {
>  	int whitespace_error;
>  	int squelch_whitespace_errors;
>  	int applied_after_fixing_ws;
> +
> +	enum ws_error_action ws_error_action;
>  };
>  
>  static int newfd = -1;
> @@ -89,12 +98,6 @@ static const char * const apply_usage[] = {
>  	NULL
>  };
>  
> -static enum ws_error_action {
> -	nowarn_ws_error,
> -	warn_on_ws_error,
> -	die_on_ws_error,
> -	correct_ws_error
> -} ws_error_action = warn_on_ws_error;

This is a good example of a variable that needs initialization, which is turned into a field in apply_state that needs initialization. It is done here:

Show 6 quoted lines

> @@ -4738,6 +4743,7 @@ int cmd_apply(int argc, const char **argv, const char *prefix_)
>  	state.p_value = 1;
>  	state.p_context = UINT_MAX;
>  	state.squelch_whitespace_errors = 5;
> +	state.ws_error_action = warn_on_ws_error;
>  	strbuf_init(&state.root, 0);

and we already have these random initial values described here.

As I do not expect that cmd_apply() which is a moral equivalent of main() will stay to be the only one who wants to see a reasonably initialized apply_state(), I think the patch that introduced the very first version of "struct apply_state" should also introduce a helper function to initialize it, i.e.

	static void init_apply_state(struct apply_state *s,
        			     const char *prefix)
        {
		memset(s, '\0', sizeof(*s));
                s->prefix = prefix;
                s->prefix_length = s->prefix ? strlen(s->prefix) : 0;
	}

in [PATCH 7/xx].