Re: [Qemu-devel] [PATCH v2 2/2] doc: Introduce coding style for errors

[Markus Armbruster]

"Daniel P. Berrange" <address@hidden> writes:

> On Mon, Nov 23, 2015 at 07:41:24PM +0100, Lluís Vilanova wrote:
>> Gives some general guidelines for reporting errors in QEMU.
>> 
>> Signed-off-by: Lluís Vilanova <address@hidden>
>> ---
>>  HACKING |   31 +++++++++++++++++++++++++++++++
>>  1 file changed, 31 insertions(+)
>> 
>> diff --git a/HACKING b/HACKING
>> index 12fbc8a..e59bc34 100644
>> --- a/HACKING
>> +++ b/HACKING
>> @@ -157,3 +157,34 @@ painful. These are:
>>   * you may assume that integers are 2s complement representation
>>   * you may assume that right shift of a signed integer duplicates
>>     the sign bit (ie it is an arithmetic shift, not a logical shift)
>> +
>> +7. Error reporting
>> +
>> +QEMU provides two different mechanisms for reporting errors. You should use 
>> one
>> +of these mechanisms instead of manually reporting them (i.e., do not use
>> +'printf', 'exit' or 'abort').
>> +
>> +7.1. Errors in user inputs
>> +
>> +QEMU provides the functions in "include/qemu/error-report.h" to report 
>> errors
>> +related to inputs provided by the user (e.g., command line arguments or
>> +configuration files).
>> +
>> +These functions generate error messages with a uniform format that can 
>> reference
>> +a location on the offending input.
>> +
>> +7.2. Other errors
>> +
>> +QEMU provides the functions in "include/qapi/error.h" to report other types 
>> of
>> +errors (i.e., not triggered by command line arguments or configuration 
>> files).
>> +
>> +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_warn, "Error with %s", "arguments");
>> +
>> +See the "include/qapi/error.h" header for additional convenience functions 
>> and
>> +special arguments. Specially, see 'error_fatal' and 'error_abort' to show 
>> errors
>> +and immediately terminate QEMU.
>
> I don't think this "Errors in user inputs" vs "Other errors" distinction
> really makes sense. Whether an error raised in a piece of code is related
> to user input or not is almost impossible to determine in practice. So as
> a rule to follow it is not practical.
>
> AFAIK, include/qemu/error-report.h is the historical failed experiment
> in structured error reporting, while  include/qapi/error.h is the new
> preferred error reporting system that everything should be using.

Not quite :)

error-report.h predates the failed experiment.  The experiment was
actually error.h, but we've since reworked it as far as practical.  One
leftover is still clearly visible: error classes.  Their use is strongly
discouraged.

error.h is for passing around errors within QEMU.  error-report.h is for
reporting them to human users via stderr or HMP.  Reporting them via QMP
is encapsulated within the QMP monitor code.

error.h has a few convenience interfaces to report to human users.
They're implemented on top of error-report.h.

> On this basis, I'd simply say that include/qemu/error-report.h is
> legacy code that should no longer be used, and that new code should
> use include/qapi/error.h exclusively and existing code converted
> where practical.

Absolutely not :)

1. Use the simplest suitable method to communicate success / failure
within code.  Stick to common methods: non-negative / -1, non-negative /
-errno, non-null / null, Error ** parameter.

Example: when a function returns a non null-pointer on success, and it
can fail only one way (as far as the caller is concerned), returning
null on failure is just fine, and certainly simpler and a lot easier on
the eyes than Error **.

Example: when a function's callers need to report details on failure
only the function really knows, use Error **, and set suitable errors.

2. Use error-report.h to report errors to stderr or an HMP monitor.

Do not report an error when you're also passing an error for somebody
else to handle!  Leave the reporting to the place that consumes the
error you pass.