02.04.2020 8:54, Markus Armbruster wrote:
Peter Maydell <peter.mayd...@linaro.org> writes:
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?
With that, you can have
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;
}
but you have to add
ERRP_AUTO_PROPAGATE();
right at the beginning of the function.
It's a small improvement. A bigger one is
if (object_property_set_link(cpuobj, OBJECT(&s->cpu_container[i]),
"memory", errp)) {
return;
return false; you mean, assuming we converting the caller too...
}
if (object_property_set_link(cpuobj, OBJECT(s), "idau", errp)) {
return;
}
if (object_property_set_bool(cpuobj, true, "realized", errp)) {
return;
}
Hmm..
Somehow, in general, especially with long function names and long parameter
lists I prefer
ret = func(..);
if (ret < 0) {
return ret;
}
notation to moving the entire function call into if condition.. Extra level of
parentheses complicates the view of the code.
But, yes, may be for boolean functions it looks a bit strange:
ok = func(..);
if (!ok) {
return false;
}
This is item "Return value conventions" in the message you replied to.
Elsewhere in this thread, I discussed the difficulties of automating the
conversion to this style. I think I know how to automate converting the
calls to use the bool return value, but converting the functions to
return it looks hard. We could do that manually for a modest set of
frequently used functions. object.h would top my list.
Are you sure that adding a lot of boolean functions is a good idea? I somehow
feel better with more usual int functions with -errno on failure.
Bool is a good return value for functions which are boolean by nature: checks,
is something correspond to some criteria. But for reporting an error I'd prefer
-errno.
--
Best regards,
Vladimir
