On Sat, Aug 27, 2016 at 11:45 AM, Christian Couder
<christian.couder@xxxxxxxxx> wrote:
> To avoid printing anything when applying with
> `state->apply_verbosity == verbosity_silent`, let's save the
> existing warn and error routines before applying, and let's
> replace them with a routine that does nothing.
>
> Then after applying, let's restore the saved routines.
>
> Helped-by: Stefan Beller <sbeller@xxxxxxxxxx>
> Signed-off-by: Christian Couder <chriscool@xxxxxxxxxxxxx>
> ---
> apply.c | 21 ++++++++++++++++++++-
> apply.h | 8 ++++++++
> 2 files changed, 28 insertions(+), 1 deletion(-)
>
> diff --git a/apply.c b/apply.c
> index ddbb0a2..bf81b70 100644
> --- a/apply.c
> +++ b/apply.c
> @@ -112,6 +112,11 @@ void clear_apply_state(struct apply_state *state)
> /* &state->fn_table is cleared at the end of apply_patch() */
> }
>
> +static void mute_routine(const char *bla, va_list params)
Instead of 'bla' you could go with 'format' as the man page for
[f]printf puts it.
Or you could leave it empty, i.e.
static void mute_routine(const char *, va_list)
...
I personally do not mind bla, as I know that the first parameter is
supposed to be the format, but let's not add unneeded information.
(Side question: Is there a culture that doesn't recognize 'bla' as a
fill word?)
> +{
> + /* do nothing */
> +}
> +
> int check_apply_state(struct apply_state *state, int force_apply)
> {
> int is_not_gitdir = !startup_info->have_repository;
> @@ -144,6 +149,13 @@ int check_apply_state(struct apply_state *state, int force_apply)
> if (!state->lock_file)
> return error("BUG: state->lock_file should not be NULL");
>
> + if (state->apply_verbosity <= verbosity_silent) {
> + state->saved_error_routine = get_error_routine();
> + state->saved_warn_routine = get_warn_routine();
> + set_error_routine(mute_routine);
> + set_warn_routine(mute_routine);
> + }
> +
> return 0;
> }
>
> @@ -4864,7 +4876,7 @@ int apply_all_patches(struct apply_state *state,
> state->newfd = -1;
> }
>
> - return !!errs;
> + res = !!errs;
I am trying to understand this and the next chunk (they work together?)
>
> end:
> if (state->newfd >= 0) {
> @@ -4872,5 +4884,12 @@ int apply_all_patches(struct apply_state *state,
> state->newfd = -1;
> }
>
> + if (state->apply_verbosity <= verbosity_silent) {
> + set_error_routine(state->saved_error_routine);
> + set_warn_routine(state->saved_warn_routine);
> + }
> +
> + if (res > -1)
> + return res;
> return (res == -1 ? 1 : 128);
So anything > -1 is returned as is, and == -1 returns 1 and <-1 returns 128 ?
So I guess a reminder/explanation on why we need to fiddle with return codes
in the commit message would help. (I do not understand how the
verbosity influences return codes.)