24.09.2021 12:04, Kevin Wolf wrote:
ERRP_GUARD() makes debugging easier by making sure that &error_abort
still fails at the real origin of the error instead of
error_propagate().
Signed-off-by: Kevin Wolf <kw...@redhat.com>
---
qom/object.c | 7 +++----
qom/object_interfaces.c | 17 ++++++-----------
2 files changed, 9 insertions(+), 15 deletions(-)
diff --git a/qom/object.c b/qom/object.c
index e86cb05b84..6be710bc40 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -1389,7 +1389,7 @@ bool object_property_get(Object *obj, const char *name,
Visitor *v,
bool object_property_set(Object *obj, const char *name, Visitor *v,
Error **errp)
{
- Error *err = NULL;
+ ERRP_GUARD();
ObjectProperty *prop = object_property_find_err(obj, name, errp);
if (prop == NULL) {
@@ -1400,9 +1400,8 @@ bool object_property_set(Object *obj, const char *name,
Visitor *v,
error_setg(errp, QERR_PERMISSION_DENIED);
return false;
}
- prop->set(obj, v, name, prop->opaque, &err);
- error_propagate(errp, err);
- return !err;
+ prop->set(obj, v, name, prop->opaque, errp);
+ return !*errp;
}
This is OK: prop->set doesn't return value, so we have to analyze errp to make
our own return value.
bool object_property_set_str(Object *obj, const char *name,
diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c
index ad9b56b59a..80691e88cd 100644
--- a/qom/object_interfaces.c
+++ b/qom/object_interfaces.c
@@ -45,26 +45,21 @@ bool user_creatable_can_be_deleted(UserCreatable *uc)
static void object_set_properties_from_qdict(Object *obj, const QDict *qdict,
Visitor *v, Error **errp)
{
+ ERRP_GUARD();
const QDictEntry *e;
- Error *local_err = NULL;
- if (!visit_start_struct(v, NULL, NULL, 0, &local_err)) {
- goto out;
+ if (!visit_start_struct(v, NULL, NULL, 0, errp)) {
+ return;
}
for (e = qdict_first(qdict); e; e = qdict_next(qdict, e)) {
- if (!object_property_set(obj, e->key, v, &local_err)) {
+ if (!object_property_set(obj, e->key, v, errp)) {
break;
}
}
- if (!local_err) {
- visit_check_struct(v, &local_err);
+ if (!*errp) {
+ visit_check_struct(v, errp);
}
visit_end_struct(v, NULL);
-
-out:
- if (local_err) {
- error_propagate(errp, local_err);
- }
}
void object_set_properties_from_keyval(Object *obj, const QDict *qdict,
ERRP_GUARD() + dereferencing errp is good when we use functions which don't
return any value. So we want to check is it fail or not and we have to analyze
errp.
But that is not the case: all called functions follow modern recommendations of
returning some value indicating failure together with setting errp.
I think, it's better not use ERRP_GUARD where it is not needed: in ideal world,
all functions that may fail do return value, and we don't need neither error
propagation, nor dereferencing errp. And don't need ERRP_GUARD. ERRP_GUARD
will be still needed to support error_prepend()/error_append() together with
error_fatal, but again that's not the case.
Here I suggest something like this:
static void object_set_properties_from_qdict(Object *obj, const QDict *qdict,
Visitor *v, Error **errp)
{
const QDictEntry *e;
if (!visit_start_struct(v, NULL, NULL, 0, errp)) {
return;
}
for (e = qdict_first(qdict); e; e = qdict_next(qdict, e)) {
if (!object_property_set(obj, e->key, v, errp)) {
goto out;
}
}
visit_check_struct(v, errp);
out:
visit_end_struct(v, NULL);
}
--
Best regards,
Vladimir
