Re: [PATCH v2 60/94] apply: make init_apply_state() return -1 instead of exit()ing

From: Eric Sunshine <sunshine@sunshineco.com>
Date: May 16, 2016, 03:37 UTC
Message-ID: <CAPig+cQ7zqhyFW5jwX5fFNKB11=hgOrOvaD-pNckxEMxmvBcjg@mail.gmail.com>
In-Reply-To: <20160511131745.2914-61-chriscool@tuxfamily.org>

On Wed, May 11, 2016 at 9:17 AM, Christian Couder <christian.couder@gmail.com> wrote:

Show 6 quoted lines

> To libify `git apply` functionality we have to signal errors to the
> caller instead of exit()ing.
>
> To do that in a compatible manner with the rest of the error handling
> in "builtin/apply.c", init_apply_state() should return -1 using
> error() instead of calling exit().

This commit message seems to be lying. It says that the -1 comes from error(), however, init_apply_state() just returns literal -1 in all cases.

By the way, mentioning "return -1 using error()" in all of these commit messages (not just this one) may be overkill. The fact that that side-effect of error() is being used in some of these cases is an implementation detail which doesn't necessarily merit mention in the commit message.

Show 63 quoted lines

> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
> ---
> diff --git a/apply.c b/apply.c
> index 508ea64..1e2b802 100644
> --- a/apply.c
> +++ b/apply.c
> @@ -55,9 +55,9 @@ int parse_ignorewhitespace_option(struct apply_state *state,
>         return error(_("unrecognized whitespace ignore option '%s'"), option);
>  }
>
> -void init_apply_state(struct apply_state *state,
> -                     const char *prefix,
> -                     struct lock_file *lock_file)
> +int init_apply_state(struct apply_state *state,
> +                    const char *prefix,
> +                    struct lock_file *lock_file)
>  {
>         memset(state, 0, sizeof(*state));
>         state->prefix = prefix;
> @@ -76,8 +76,9 @@ void init_apply_state(struct apply_state *state,
>
>         git_apply_config();
>         if (apply_default_whitespace && parse_whitespace_option(state, apply_default_whitespace))
> -               exit(1);
> +               return -1;
>         if (apply_default_ignorewhitespace && parse_ignorewhitespace_option(state, apply_default_ignorewhitespace))
> -               exit(1);
> +               return -1;
> +       return 0;
>  }
>
> diff --git a/apply.h b/apply.h
> index 0f77f4d..f3b2ae4 100644
> --- a/apply.h
> +++ b/apply.h
> @@ -117,8 +117,8 @@ extern int parse_whitespace_option(struct apply_state *state,
>  extern int parse_ignorewhitespace_option(struct apply_state *state,
>                                          const char *option);
>
> -extern void init_apply_state(struct apply_state *state,
> -                            const char *prefix,
> -                            struct lock_file *lock_file);
> +extern int init_apply_state(struct apply_state *state,
> +                           const char *prefix,
> +                           struct lock_file *lock_file);
>
>  #endif
> diff --git a/builtin/apply.c b/builtin/apply.c
> index 805c707..b31f9eb 100644
> --- a/builtin/apply.c
> +++ b/builtin/apply.c
> @@ -4724,7 +4724,8 @@ int cmd_apply(int argc, const char **argv, const char *prefix)
>                 OPT_END()
>         };
>
> -       init_apply_state(&state, prefix, NULL);
> +       if (init_apply_state(&state, prefix, NULL))
> +               exit(1);
>
>         argc = parse_options(argc, argv, state.prefix, builtin_apply_options,
>                         apply_usage, 0);
> --
> 2.8.2.490.g3dabe57