On Wed, May 11, 2016 at 9:17 AM, Christian Couder
<christian.couder@xxxxxxxxx> wrote:
> 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.
> Signed-off-by: Christian Couder <chriscool@xxxxxxxxxxxxx>
> ---
> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html