Re: [PATCH v13 10/14] apply: change error_routine when silent
On Sun, Sep 4, 2016 at 6:31 PM, Ramsay Jones wrote: > > > On 04/09/16 11:54, Christian Couder wrote: >> On Thu, Sep 1, 2016 at 10:19 AM, Christian Couder >> wrote: >>> On Thu, Sep 1, 2016 at 12:20 AM, Stefan Beller wrote: > > +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) ... >>> >>> Ok to do that. >> >> Actually I get the following error when doing that: >> >> apply.c: In function ‘mute_routine’: >> apply.c:115:1: error: parameter name omitted >> static void mute_routine(const char *, va_list) >> ^ >> apply.c:115:1: error: parameter name omitted >> make: *** [apply.o] Error 1 > > Yes, this is not C++. ;-) > >> So I will leave it as is. > > I think I would prefer to see: > > static void mute_routine(const char *msg, va_list params) > > given that it would either be an error-msg or a warning-msg. Ok for "msg".
Re: [PATCH v13 10/14] apply: change error_routine when silent
On 04/09/16 11:54, Christian Couder wrote: > On Thu, Sep 1, 2016 at 10:19 AM, Christian Couder > wrote: >> On Thu, Sep 1, 2016 at 12:20 AM, Stefan Beller wrote: >>> On Sat, Aug 27, 2016 at 11:45 AM, Christian Couder >>> 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 Signed-off-by: Christian Couder --- 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) >>> ... >> >> Ok to do that. > > Actually I get the following error when doing that: > > apply.c: In function ‘mute_routine’: > apply.c:115:1: error: parameter name omitted > static void mute_routine(const char *, va_list) > ^ > apply.c:115:1: error: parameter name omitted > make: *** [apply.o] Error 1 Yes, this is not C++. ;-) > So I will leave it as is. I think I would prefer to see: static void mute_routine(const char *msg, va_list params) given that it would either be an error-msg or a warning-msg. ATB, Ramsay Jones
Re: [PATCH v13 10/14] apply: change error_routine when silent
On Thu, Sep 1, 2016 at 10:19 AM, Christian Couder wrote: > On Thu, Sep 1, 2016 at 12:20 AM, Stefan Beller wrote: >> On Sat, Aug 27, 2016 at 11:45 AM, Christian Couder >> 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 >>> Signed-off-by: Christian Couder >>> --- >>> 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) >> ... > > Ok to do that. Actually I get the following error when doing that: apply.c: In function ‘mute_routine’: apply.c:115:1: error: parameter name omitted static void mute_routine(const char *, va_list) ^ apply.c:115:1: error: parameter name omitted make: *** [apply.o] Error 1 So I will leave it as is.
Re: [PATCH v13 10/14] apply: change error_routine when silent
On Thu, Sep 1, 2016 at 12:20 AM, Stefan Beller wrote:
> On Sat, Aug 27, 2016 at 11:45 AM, Christian Couder
> 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
>> Signed-off-by: Christian Couder
>> ---
>> 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)
> ...
Ok to do that.
> 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?)
Yes, 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.)
It doesn't influence return codes, but we need to restore error
routines just before returning, so we cannot return early anymore.
I will add something to the commit message.
Re: [PATCH v13 10/14] apply: change error_routine when silent
On Sat, Aug 27, 2016 at 11:45 AM, Christian Couder
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
> Signed-off-by: Christian Couder
> ---
> 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.)
[PATCH v13 10/14] apply: change error_routine when silent
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
Signed-off-by: Christian Couder
---
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)
+{
+ /* 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;
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);
}
diff --git a/apply.h b/apply.h
index bd4eb6d..5cde641 100644
--- a/apply.h
+++ b/apply.h
@@ -94,6 +94,14 @@ struct apply_state {
*/
struct string_list fn_table;
+ /*
+* This is to save reporting routines before using
+* set_error_routine() or set_warn_routine() to install muting
+* routines when in verbosity_silent mode.
+*/
+ void (*saved_error_routine)(const char *err, va_list params);
+ void (*saved_warn_routine)(const char *warn, va_list params);
+
/* These control whitespace errors */
enum apply_ws_error_action ws_error_action;
enum apply_ws_ignore ws_ignore_action;
--
2.9.2.770.g14ff7d2
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
