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

From: Junio C Hamano <[email-masked]>
Date: Tue, 26 Apr 2016 13:36:04 -0700

Christian Couder <[email-masked]> writes:

> +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:

> @@ -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].