qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Questionable aspects of QEMU Error's design


From: Vladimir Sementsov-Ogievskiy
Subject: Re: Questionable aspects of QEMU Error's design
Date: Thu, 2 Apr 2020 08:31:34 +0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.2.1

01.04.2020 23:15, Peter Maydell wrote:
On Wed, 1 Apr 2020 at 10:03, Markus Armbruster <address@hidden> wrote:

QEMU's Error was patterned after GLib's GError.  Differences include:

 From my POV the major problem with Error as we have it today
is that it makes the simple process of writing code like
device realize functions horrifically boilerplate heavy;
for instance this is from hw/arm/armsse.c:

         object_property_set_link(cpuobj, OBJECT(&s->cpu_container[i]),
                                  "memory", &err);
         if (err) {
             error_propagate(errp, err);
             return;
         }
         object_property_set_link(cpuobj, OBJECT(s), "idau", &err);
         if (err) {
             error_propagate(errp, err);
             return;
         }
         object_property_set_bool(cpuobj, true, "realized", &err);
         if (err) {
             error_propagate(errp, err);
             return;
         }

16 lines of code just to set 2 properties on an object
and realize it. It's a lot of boilerplate and as
a result we frequently get it wrong or take shortcuts
(eg forgetting the error-handling entirely, calling
error_propagate just once for a whole sequence of
calls, taking the lazy approach and using err_abort
or err_fatal when we ought really to be propagating
an error, etc). I haven't looked at 'auto propagation'
yet, hopefully it will help?

Yes, after it the code above will look like this:

... some_func(..., errp)
{
    ERRP_AUTO_PROPAGATE(); # magic macro at function start, and no "Error *err" 
definition

...
          object_property_set_link(cpuobj, OBJECT(&s->cpu_container[i]),
                                   "memory", errp);
          if (*errp) {
              return;
          }
          object_property_set_link(cpuobj, OBJECT(s), "idau", errp);
          if (*errp) {
              return;
          }
          object_property_set_bool(cpuobj, true, "realized", errp);
          if (*errp) {
              return;
          }
...
}

 - propagation is automatic, errp is used directly and may be safely 
dereferenced.



--
Best regards,
Vladimir



reply via email to

[Prev in Thread] Current Thread [Next in Thread]