[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 ] doc: Introduce coding style for errors
From: |
Lluís Vilanova |
Subject: |
Re: [Qemu-devel] [PATCH v3 ] doc: Introduce coding style for errors |
Date: |
Wed, 27 Jan 2016 20:06:12 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) |
Thomas Huth writes:
> On 20.01.2016 15:10, Lluís Vilanova wrote:
>> Thomas Huth writes:
>>
>>> On 18.01.2016 21:26, Eric Blake wrote:
>>>> On 01/15/2016 06:54 AM, Lluís Vilanova wrote:
>>>>> Gives some general guidelines for reporting errors in QEMU.
>>>>>
>>>>> Signed-off-by: Lluís Vilanova <address@hidden>
>>>>> ---
>>>>> HACKING | 36 ++++++++++++++++++++++++++++++++++++
>>>>> 1 file changed, 36 insertions(+)
>>> ...
>>>>> +Functions in this header are used to accumulate error messages in an
>>>>> 'Error'
>>>>> +object, which can be propagated up the call chain where it is finally
>>>>> reported.
>>>>> +
>>>>> +In its simplest form, you can immediately report an error with:
>>>>> +
>>>>> + error_setg(&error_fatal, "Error with %s", "arguments");
>>>>
>>>> This paradigm doesn't appear anywhere in the current code base
>>>> (hw/ppc/spapr*.c has a few cases of error_setg(&error_abort), but
>>>> nothing directly passes error_fatal). It's a bit odd to document
>>>> something that isn't actually used.
>>
>>> +1 for _not_ documenting this here: IMHO this looks ugly. If we want
>>> something like this, I think we should introduce a proper
>>> error_report_fatal() function instead.
>>
>> That's a bit of a chicken and egg problem. My main intention was to provide a
>> best practices summary on reporting messages/errors, since QEMU's code is
>> really
>> heterogeneous on that regard. But there seems to be no consensus on some
>> details
>> of what the proper way should be with the current interfaces.
>>
>> Utility functions for "regular messages", warnings, fatals and aborts would
>> definitiely be an improvement IMHO, but I dont have time to adapt existing
>> code
>> to these (and I was told not to add unused utility functions for this).
>>
>> Now, if I were able to add such functions, it'd be something like:
>>
>> // Generate message "as is"; not sure if this should exist.
>> message_report(fmt, ...)
> Not sure what this should be good for? We've already got error_report()
> that generates messages "as is", haven't we?
Well, it just seemed wrong to me using error_report() to report "regular
messages" :)
>> // Generate message with prepended file/line information for the caller.
>> // Calls exit/abort on the last two.
>> error_report_{warn,fatal,abort}(fmt, ...)
>>
>> // Same with an added message from strerror.
>> error_report_{warn,fatal,abort}_errno(fmt, ...)
>>
>> But, should I add these without providing extensive patches that refactor
>> code
>> to use them?
> Maybe create a patch that introduces the _fatal and _abort functions
> (I'd skip the _warn functions for now), and use them in one or two files
> (e.g. replace the error_setg(&error_abort, ...) in spapr.c). That should
> not be that much of work, and could be a good base for further discussion?
I can do that. But then should 'error_fatal' and 'error_abort' be officially
deprecated in favour of error_report_fatal() and error_report_abort()?
Cheers,
Lluis
- [Qemu-devel] [RFC][PATCH v3 ] utils: Improve and document error reporting, Lluís Vilanova, 2016/01/15
- [Qemu-devel] [PATCH v3 ] doc: Introduce coding style for errors, Lluís Vilanova, 2016/01/15
- Re: [Qemu-devel] [PATCH v3 ] doc: Introduce coding style for errors, Eric Blake, 2016/01/18
- Re: [Qemu-devel] [PATCH v3 ] doc: Introduce coding style for errors, Thomas Huth, 2016/01/19
- Re: [Qemu-devel] [PATCH v3 ] doc: Introduce coding style for errors, Lluís Vilanova, 2016/01/20
- Re: [Qemu-devel] [PATCH v3 ] doc: Introduce coding style for errors, Thomas Huth, 2016/01/27
- Re: [Qemu-devel] [PATCH v3 ] doc: Introduce coding style for errors,
Lluís Vilanova <=
- Re: [Qemu-devel] [PATCH v3 ] doc: Introduce coding style for errors, Lluís Vilanova, 2016/01/27
- Re: [Qemu-devel] [PATCH v3 ] doc: Introduce coding style for errors, Thomas Huth, 2016/01/28
- Re: [Qemu-devel] [PATCH v3 ] doc: Introduce coding style for errors, Lluís Vilanova, 2016/01/28