Re: [PATCH v13 10/14] apply: change error_routine when silent

> On Sat, Aug 27, 2016 at 11:45 AM, Christian Couder
> <christian.couder@gmail.com> 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@google.com>
>> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
>> ---
>>  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?)
>
>> @@ -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.)