Markus Armbruster <arm...@redhat.com> 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.
