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

On Tue, Apr 26, 2016 at 10:36 PM, Junio C Hamano [off-list ref] wrote:

Christian Couder [off-list ref] writes:

quoted

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

quoted

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

Yeah, Eric also made nearly the same comment, so in the next reroll
the init_apply_state() is introduced in the patch immediately after
the patch that creates 'struct apply_state' (rather than much later in
the series). I prefer to not introduce it in the same patch as the
patch that creates 'struct apply_state' is already quite big. (But if
you insist, yeah I will squash both patches together.)