[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Questionable aspects of QEMU Error's design
From: |
Markus Armbruster |
Subject: |
Re: Questionable aspects of QEMU Error's design |
Date: |
Sat, 04 Apr 2020 12:59:27 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux) |
Markus Armbruster <address@hidden> writes:
> Markus Armbruster <address@hidden> writes:
>
>> QEMU's Error was patterned after GLib's GError. Differences include:
> [...]
>> * Return value conventions
>>
>> Common: non-void functions return a distinct error value on failure
>> when such a value can be defined. Patterns:
>>
>> - Functions returning non-null pointers on success return null pointer
>> on failure.
>>
>> - Functions returning non-negative integers on success return a
>> negative error code on failure.
>>
>> Different: GLib discourages void functions, because these lead to
>> awkward error checking code. We have tons of them, and tons of
>> awkward error checking code:
>>
>> Error *err = NULL;
>> frobnicate(arg, &err);
>> if (err) {
>> ... recover ...
>> error_propagate(errp, err);
>> }
>>
>> instead of
>>
>> if (!frobnicate(arg, errp))
>> ... recover ...
>> }
>>
>> Can also lead to pointless creation of Error objects.
>>
>> I consider this a design mistake. Can we still fix it? We have more
>> than 2000 void functions taking an Error ** parameter...
>>
>> Transforming code that receives and checks for errors with Coccinelle
>> shouldn't be hard. Transforming code that returns errors seems more
>> difficult. We need to transform explicit and implicit return to
>> either return true or return false, depending on what we did to the
>> @errp parameter on the way to the return. Hmm.
> [...]
>
> To figure out what functions with an Error ** parameter return, I used
> Coccinelle to find such function definitions and print the return types.
> Summary of results:
>
> 2155 void
> 873 signed integer
> 494 pointer
> 153 bool
> 33 unsigned integer
> 6 enum
> ---------------------
> 3714 total
>
> I then used Coccinelle to find checked calls of void functions (passing
> &error_fatal or &error_abort is not considered "checking" here). These
> calls become simpler if we make the functions return a useful value. I
> found a bit under 600 direct calls, and some 50 indirect calls.
>
> Most frequent direct calls:
>
> 127 object_property_set_bool
> 27 qemu_opts_absorb_qdict
> 16 visit_type_str
> 14 visit_type_int
> 10 visit_type_uint32
>
> Let's have a closer look at object_property_set() & friends. Out of
> almost 1000 calls, some 150 are checked. While I'm sure many of the
> unchecked calls can't actually fail, I am concerned some unchecked calls
> can.
>
> If we adopt the convention to return a value that indicates success /
> failure, we should consider converting object.h to it sooner rather than
> later.
>
> Please understand these are rough numbers from quick & dirty scripts.
Paolo, Daniel, Eduardo,
Please pick one for QOM:
* Do nothing. Looks like
object_property_set_bool(..., &err);
if (err) {
error_propagate(errp, err);
return;
}
* Return true on success, false on error. Looks like
if (!object_property_set_bool(..., errp)) {
return;
}
* Return 0 on success, -1 on error. Looks like
if (object_property_set_bool(..., errp) < 0) {
return;
}
This is slightly more likely to require line wrapping than the
previous one.
- Re: Questionable aspects of QEMU Error's design, (continued)
- Re: Questionable aspects of QEMU Error's design, Markus Armbruster, 2020/04/02
- Re: Questionable aspects of QEMU Error's design, Vladimir Sementsov-Ogievskiy, 2020/04/02
- Re: Questionable aspects of QEMU Error's design, Markus Armbruster, 2020/04/02
- Re: Questionable aspects of QEMU Error's design, Vladimir Sementsov-Ogievskiy, 2020/04/02
- Re: Questionable aspects of QEMU Error's design, Markus Armbruster, 2020/04/03
- Re: Questionable aspects of QEMU Error's design, Paolo Bonzini, 2020/04/02
- Re: Questionable aspects of QEMU Error's design, Daniel P . Berrangé, 2020/04/02
- Re: Questionable aspects of QEMU Error's design, Alex Bennée, 2020/04/02
- Re: Questionable aspects of QEMU Error's design, Eric Blake, 2020/04/02
Re: Questionable aspects of QEMU Error's design, Markus Armbruster, 2020/04/04
Re: Questionable aspects of QEMU Error's design, Markus Armbruster, 2020/04/27