02.04.2020 12:36, BALATON Zoltan wrote:
On Thu, 2 Apr 2020, Vladimir Sementsov-Ogievskiy wrote:
01.04.2020 23:15, Peter Maydell wrote:
On Wed, 1 Apr 2020 at 10:03, Markus Armbruster <arm...@redhat.com> 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.
Not much better. Could it be something like:
Actually, much better, as it solves some real problems around error propagation.
ERRP_RET(object_property_set_link(cpuobj, OBJECT(&s->cpu_container[i]),
"memory", errp));
ERRP_RET(object_property_set_link(cpuobj, OBJECT(s), "idau", errp));
ERRP_RET(object_property_set_bool(cpuobj, true, "realized", errp));
and turn all
ret = func(...);
if (ret < 0) {
return ret;
}
into
FAIL_RET(func(...))
?
Not a problem to make such macro.. But I think it's a bad idea to turn all the
code
into sequence of macro invocations. It's hard to debug and follow.
--
Best regards,
Vladimir
