Re: Questionable aspects of QEMU Error's design

[Markus Armbruster]

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.