[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v6 1/5] util: Introduce error reporting function
From: |
Thomas Huth |
Subject: |
Re: [Qemu-devel] [PATCH v6 1/5] util: Introduce error reporting functions with fatal/abort |
Date: |
Wed, 3 Feb 2016 10:58:52 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.0 |
On 03.02.2016 10:48, Markus Armbruster wrote:
> David Gibson <address@hidden> writes:
>
>> On Tue, Feb 02, 2016 at 10:47:35PM +0100, Thomas Huth wrote:
>>> On 02.02.2016 19:53, Markus Armbruster wrote:
>>>> Lluís Vilanova <address@hidden> writes:
>>> ...
>>>
>>>>> diff --git a/include/qemu/error-report.h b/include/qemu/error-report.h
>>>>> index 7ab2355..6c2f142 100644
>>>>> --- a/include/qemu/error-report.h
>>>>> +++ b/include/qemu/error-report.h
>>>>> @@ -43,4 +43,23 @@ void error_report(const char *fmt, ...)
>>>>> GCC_FMT_ATTR(1, 2);
>>>>> const char *error_get_progname(void);
>>>>> extern bool enable_timestamp_msg;
>>>>>
>>>>> +/* Report message and exit with error */
>>>>> +void QEMU_NORETURN error_vreport_fatal(const char *fmt, va_list ap)
>>>>> GCC_FMT_ATTR(1, 0);
>>>>> +void QEMU_NORETURN error_report_fatal(const char *fmt, ...)
>>>>> GCC_FMT_ATTR(1, 2);
>>>>
>>>> This lets people write things like
>>>>
>>>> error_report_fatal("The sky is falling");
>>>>
>>>> instead of
>>>>
>>>> error_report("The sky is falling");
>>>> exit(1);
>>>>
>>>> or
>>>>
>>>> fprintf(stderr, "The sky is falling\n");
>>>> exit(1);
>>>>
>>>> I don't think that's an improvement in clarity.
>>>
>>> The problem is not the existing code, but that in a couple of new
>>> patches, I've now already seen that people are trying to use
>>>
>>> error_setg(&error_fatal, ... );
>>
>> So, I don't actually see any real advantage to error_report_fatal(...)
>> over error_setg(&error_fatal, ...).
>
> I do. Compare:
>
> (a) error_report(...);
> exit(1);
>
> (b) error_report_fatal(...);
>
> (c) error_setg(&error_fatal, ...);
>
> In my opinion, (a) is clearest: even a relatively clueless reader will
> know what exit(1) does, can guess what error_report() approximately
> does, and doesn't need to know what it does exactly. (b) is slightly
> less obvious, and (c) is positively opaque.
>
> Let's stick to the obvious (a) and be done with it.
Ok, (a) is fine for me too, as long as we avoid (c). Lluís, could you
maybe add that information to your patch that updates the HACKING text?
(and sorry for the fuzz with error_report_fatal() ... I thought it would
be a good solution to avoid (c), but if (a) is preferred instead, then
we should go with that solution instead).
And, by the way, what about the spots that currently already use
error_setg(&error_abort, ....) ? Should they be turned into
error_report() + abort() instead? Or only abort(), without error
message, since abort() is only about programming errors?
Thomas
- [Qemu-devel] [RFC][PATCH v6 0/5] utils: Improve and document error reporting, Lluís Vilanova, 2016/02/02
- [Qemu-devel] [PATCH v6 1/5] util: Introduce error reporting functions with fatal/abort, Lluís Vilanova, 2016/02/02
- Re: [Qemu-devel] [PATCH v6 1/5] util: Introduce error reporting functions with fatal/abort, Markus Armbruster, 2016/02/02
- Re: [Qemu-devel] [PATCH v6 1/5] util: Introduce error reporting functions with fatal/abort, Thomas Huth, 2016/02/02
- Re: [Qemu-devel] [PATCH v6 1/5] util: Introduce error reporting functions with fatal/abort, David Gibson, 2016/02/03
- Re: [Qemu-devel] [PATCH v6 1/5] util: Introduce error reporting functions with fatal/abort, Markus Armbruster, 2016/02/03
- Re: [Qemu-devel] [PATCH v6 1/5] util: Introduce error reporting functions with fatal/abort,
Thomas Huth <=
- Re: [Qemu-devel] [PATCH v6 1/5] util: Introduce error reporting functions with fatal/abort, Markus Armbruster, 2016/02/03
- Re: [Qemu-devel] [PATCH v6 1/5] util: Introduce error reporting functions with fatal/abort, Lluís Vilanova, 2016/02/03
- Re: [Qemu-devel] [PATCH v6 1/5] util: Introduce error reporting functions with fatal/abort, Markus Armbruster, 2016/02/03
- Re: [Qemu-devel] [PATCH v6 1/5] util: Introduce error reporting functions with fatal/abort, Lluís Vilanova, 2016/02/03
- Re: [Qemu-devel] [PATCH v6 1/5] util: Introduce error reporting functions with fatal/abort, Markus Armbruster, 2016/02/03
- Re: [Qemu-devel] [PATCH v6 1/5] util: Introduce error reporting functions with fatal/abort, David Gibson, 2016/02/03
- Re: [Qemu-devel] [PATCH v6 1/5] util: Introduce error reporting functions with fatal/abort, Markus Armbruster, 2016/02/03
- Re: [Qemu-devel] [PATCH v6 1/5] util: Introduce error reporting functions with fatal/abort, Lluís Vilanova, 2016/02/03
[Qemu-devel] [PATCH v6 2/5] util: Use new error_report_fatal/abort instead of error_setg(&error_fatal/abort), Lluís Vilanova, 2016/02/02