[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH build-fix v1 1/1] error: Don't use error_report(
From: |
Peter Crosthwaite |
Subject: |
Re: [Qemu-devel] [PATCH build-fix v1 1/1] error: Don't use error_report() for assertion msgs. |
Date: |
Wed, 15 Jan 2014 13:34:28 +1000 |
On Wed, Jan 15, 2014 at 1:31 PM, Peter Crosthwaite
<address@hidden> wrote:
> On Wed, Jan 15, 2014 at 12:55 PM, Andreas Färber <address@hidden> wrote:
>> Am 15.01.2014 03:29, schrieb Peter Crosthwaite:
>>> Use fprintf(stderr instead. This removes dependency of libqemuutil.a
>>> on the monitor.
>>>
>>> We can further justify this change, in that this code path should only
>>> trigger under a fatal error condition. fprintf-stderr is probably the
>>> appropriate medium as under a fatal error conidition the monitor itself
>>> may be down and out for the count. So assertion failure messages should
>>> go lowest common denominator - straight to stderr.
>>
>> Actually I thought the point of error_report() was to add an appropriate
>> prefix "qemu-system-foo: ..." to the error message and an optional
>> timestamp, not to send it to the monitor...
>>
>
> Well this patch routes it away from the monitor. Never implemented
> prefixing though. My main motivations were:
>
> 1: Text reduction. No need to define local_err and assert them constantly.
> 2: Full backtraceability of the error.
>
Sry misread your mail, you were commenting error_report not error_abort as
I read it if your wondering why my mail makes no sense at all. Sry for the
noise :S
> Regards,
> Peter
>
>>>
>>> Fixes the build as reported by Kevin Wolf. Issue debugged and change
>>> suggested by Luiz Capitulino. Issue introduced by
>>> 5d24ee70bcbcf578614193526bcd5ed30a8eb16c.
>>>
>>> Signed-off-by: Peter Crosthwaite <address@hidden>
>>> ---
>>>
>>> util/error.c | 8 ++++----
>>> 1 file changed, 4 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/util/error.c b/util/error.c
>>> index f11f1d5..7c7650c 100644
>>> --- a/util/error.c
>>> +++ b/util/error.c
>>> @@ -44,7 +44,7 @@ void error_set(Error **errp, ErrorClass err_class, const
>>> char *fmt, ...)
>>> err->err_class = err_class;
>>>
>>> if (errp == &error_abort) {
>>> - error_report("%s", error_get_pretty(err));
>>> + fprintf(stderr, "%s", error_get_pretty(err));
>>
>> You need to add \n if you do this.
>>
Fixed. V2 en-route.
Regards,
Peter
>> Andreas
>>
>>> abort();
>>> }
>>>
>>> @@ -80,7 +80,7 @@ void error_set_errno(Error **errp, int os_errno,
>>> ErrorClass err_class,
>>> err->err_class = err_class;
>>>
>>> if (errp == &error_abort) {
>>> - error_report("%s", error_get_pretty(err));
>>> + fprintf(stderr, "%s", error_get_pretty(err));
>>> abort();
>>> }
>>>
>>> @@ -125,7 +125,7 @@ void error_set_win32(Error **errp, int win32_err,
>>> ErrorClass err_class,
>>> err->err_class = err_class;
>>>
>>> if (errp == &error_abort) {
>>> - error_report("%s", error_get_pretty(err));
>>> + fprintf(stderr, "%s", error_get_pretty(err));
>>> abort();
>>> }
>>>
>>> @@ -171,7 +171,7 @@ void error_free(Error *err)
>>> void error_propagate(Error **dst_err, Error *local_err)
>>> {
>>> if (local_err && dst_err == &error_abort) {
>>> - error_report("%s", error_get_pretty(local_err));
>>> + fprintf(stderr, "%s", error_get_pretty(local_err));
>>> abort();
>>> } else if (dst_err && !*dst_err) {
>>> *dst_err = local_err;
>>>
>>
>>
>> --
>> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
>> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
>>