On 12/5/19 8:48 AM, Vladimir Sementsov-Ogievskiy wrote:
@@ -918,27 +917,26 @@ static void device_set_realized(Object *obj, bool value, Error **errp) }} else if (!value && dev->realized) {- Error **local_errp = NULL; + /* We want local_err to track only the first error */ QLIST_FOREACH(bus, &dev->child_bus, sibling) { - local_errp = local_err ? NULL : &local_err; object_property_set_bool(OBJECT(bus), false, "realized", - local_errp); + local_err ? NULL : &local_err); }This is a rather unusual way to keep the first error of several.
It may be unusual, but has the benefit of avoiding error_propagate...
qapi/error.h advises: * Receive and accumulate multiple errors (first one wins): * Error *err = NULL, *local_err = NULL; * foo(arg, &err); * bar(arg, &local_err); * error_propagate(&err, local_err); * if (err) { * handle the error... * }Hmm, honestly, I like more what I've written: 1. less code 2. logic is more clean: we store first error to local_err, and after first error pass NULL as a parameter. No propagation or extra error variables. 3. more efficient (no propagation, no extra allocation for errors which we'll drop anyway) (I understand that efficiency of error path is not thing to care about, so it's at third place) Also, propagation which you propose is also unusual thing (it proposed in comment, but who reads it :). I've never seen it before, and I've to go and check that error_propagate works correctly when first argument is already set. So, I'd prefer to keep now this patch as is, and to convert later if we really need it.If replacing this by the usual way is too troublesome now, we can do it after the ERRP_AUTO_PROPAGATE() conversion. Your choice.
...and after conversion to use ERRP_AUTO_PROPATATE(), the use of error_propagate() should NOT occur in any code _except_ for the macro definition (any other use of the function points out a place where we failed to use the macro to get rid of boilerplate).
-- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
