[PULL 27/29] qom: fix ability to create objects without a parent
object_new_with_propv allowed id/parent to be optional, in which case
the caller was expected to own the returned object. Unfortunately a
trailing object_unref() meant that the returned object was already
freed.
It is confusing to have a single method with two different ownership
scenarios for the returned object.
Make id/parent mandatory in object_new_with_propv once more, and add
a new object_new_with_propv_parentless that does not accept id/parent
at all and lets the caller own the returned reference.
The helper method has abstracted the way properties are represented
and set in order to facilitate the subsequent commit.
Unit tests are added to address the root cause that allowed the bug
to slip through in commit 6134d752.
Fixes: 6134d7522e5 ("qom: don't require user creatable objects to be
registered")
Tested-by: Philippe Mathieu-Daudé
Reviewed-by: Marc-André Lureau
Signed-off-by: Daniel P. Berrangé
---
include/qom/object.h| 47 ++
qom/object.c| 74 +++
tests/unit/check-qom-proplist.c | 88 -
3 files changed, 187 insertions(+), 22 deletions(-)
diff --git a/include/qom/object.h b/include/qom/object.h
index fb1cd92d97..11f55613fc 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -719,6 +719,53 @@ Object *object_new_with_props_from_qdict(const char
*typename,
Visitor *v,
Error **errp);
+/**
+ * object_new_with_props_parentless:
+ * @typename: The name of the type of the object to instantiate.
+ * @errp: pointer to error object
+ * @...: list of property names and values
+ *
+ * Behaviour as object_new_with_props(), except the object
+ * will not be added to any parent and thus the caller will
+ * own the returned instance. The caller must call
+ * object_unref when it is no longer required.
+ */
+Object *object_new_with_props_parentless(const char *typename,
+ Error **errp,
+ ...) G_GNUC_NULL_TERMINATED;
+
+/**
+ * object_new_with_propv_parentless:
+ * @typename: The name of the type of the object to instantiate.
+ * @vargs: list of property names and values
+ * @errp: pointer to error object
+ *
+ * Behaviour as object_new_with_propv(), except the object
+ * will not be added to any parent and thus the caller will
+ * own the returned instance. The caller must call
+ * object_unref when it is no longer required.
+ */
+Object *object_new_with_propv_parentless(const char *typename,
+ va_list vargs,
+ Error **errp);
+
+/**
+ * object_new_with_props_from_qdict_parentless:
+ * @typename: The name of the type of the object to instantiate.
+ * @props: dictionary of property names and values
+ * @v: visitor to iterate over @props
+ * @errp: pointer to error object
+ *
+ * Behaviour as object_new_with_props_from_qdict(), except the
+ * object will not be added to any parent and thus the caller
+ * will own the returned instance. The caller must call
+ * object_unref when it is no longer required.
+ */
+Object *object_new_with_props_from_qdict_parentless(const char *typename,
+const QDict *props,
+Visitor *v,
+Error **errp);
+
/**
* object_set_props:
* @obj: the object instance to set properties on
diff --git a/qom/object.c b/qom/object.c
index a11f559445..5a7ac457c1 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -740,6 +740,8 @@ Object *object_new_with_props(const char *typename,
va_list vargs;
Object *obj;
+assert(parent != NULL);
+assert(id != NULL);
va_start(vargs, errp);
obj = object_new_with_propv(typename, parent, id, vargs, errp);
va_end(vargs);
@@ -757,10 +759,14 @@ object_new_with_props_helper(const char *typename,
Error **errp),
Error **errp)
{
+ERRP_GUARD();
Object *obj;
ObjectClass *klass;
UserCreatable *uc;
+assert((id != NULL && parent != NULL) ||
+ (id == NULL && parent == NULL));
+
if (id != NULL && !id_wellformed(id)) {
error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "id", "an identifier");
error_append_hint(errp, "Identifiers consist of letters, digits, "
@@ -785,7 +791,10 @@ object_new_with_props_helper(const char *typename,
}
if (id != NULL) {
-object_property_add_child(parent, id, obj);
+object_property_try_add_child(parent, id, obj, errp);
+if (*errp) {
+goto error;
+}
}
uc = (UserCreatable *)object_dynamic_cast(obj, TYPE_USER_CREATABLE);
@@ -798,7 +807,6 @@ object_new_with_props_helper(const char *typename,
}
}
[PULL 27/29] qom: fix ability to create objects without a parent
object_new_with_propv allowed id/parent to be optional, in which case
the caller was expected to own the returned object. Unfortunately a
trailing object_unref() meant that the returned object was already
freed.
It is confusing to have a single method with two different ownership
scenarios for the returned object.
Make id/parent mandatory in object_new_with_propv once more, and add
a new object_new_with_propv_parentless that does not accept id/parent
at all and lets the caller own the returned reference.
The helper method has abstracted the way properties are represented
and set in order to facilitate the subsequent commit.
Unit tests are added to address the root cause that allowed the bug
to slip through in commit 6134d752.
Fixes: 6134d7522e5 ("qom: don't require user creatable objects to be
registered")
Tested-by: Philippe Mathieu-Daudé
Reviewed-by: Marc-André Lureau
Signed-off-by: Daniel P. Berrangé
---
include/qom/object.h| 47 ++
qom/object.c| 74 +++
tests/unit/check-qom-proplist.c | 88 -
3 files changed, 187 insertions(+), 22 deletions(-)
diff --git a/include/qom/object.h b/include/qom/object.h
index fb1cd92d97..11f55613fc 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -719,6 +719,53 @@ Object *object_new_with_props_from_qdict(const char
*typename,
Visitor *v,
Error **errp);
+/**
+ * object_new_with_props_parentless:
+ * @typename: The name of the type of the object to instantiate.
+ * @errp: pointer to error object
+ * @...: list of property names and values
+ *
+ * Behaviour as object_new_with_props(), except the object
+ * will not be added to any parent and thus the caller will
+ * own the returned instance. The caller must call
+ * object_unref when it is no longer required.
+ */
+Object *object_new_with_props_parentless(const char *typename,
+ Error **errp,
+ ...) G_GNUC_NULL_TERMINATED;
+
+/**
+ * object_new_with_propv_parentless:
+ * @typename: The name of the type of the object to instantiate.
+ * @vargs: list of property names and values
+ * @errp: pointer to error object
+ *
+ * Behaviour as object_new_with_propv(), except the object
+ * will not be added to any parent and thus the caller will
+ * own the returned instance. The caller must call
+ * object_unref when it is no longer required.
+ */
+Object *object_new_with_propv_parentless(const char *typename,
+ va_list vargs,
+ Error **errp);
+
+/**
+ * object_new_with_props_from_qdict_parentless:
+ * @typename: The name of the type of the object to instantiate.
+ * @props: dictionary of property names and values
+ * @v: visitor to iterate over @props
+ * @errp: pointer to error object
+ *
+ * Behaviour as object_new_with_props_from_qdict(), except the
+ * object will not be added to any parent and thus the caller
+ * will own the returned instance. The caller must call
+ * object_unref when it is no longer required.
+ */
+Object *object_new_with_props_from_qdict_parentless(const char *typename,
+const QDict *props,
+Visitor *v,
+Error **errp);
+
/**
* object_set_props:
* @obj: the object instance to set properties on
diff --git a/qom/object.c b/qom/object.c
index a11f559445..5a7ac457c1 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -740,6 +740,8 @@ Object *object_new_with_props(const char *typename,
va_list vargs;
Object *obj;
+assert(parent != NULL);
+assert(id != NULL);
va_start(vargs, errp);
obj = object_new_with_propv(typename, parent, id, vargs, errp);
va_end(vargs);
@@ -757,10 +759,14 @@ object_new_with_props_helper(const char *typename,
Error **errp),
Error **errp)
{
+ERRP_GUARD();
Object *obj;
ObjectClass *klass;
UserCreatable *uc;
+assert((id != NULL && parent != NULL) ||
+ (id == NULL && parent == NULL));
+
if (id != NULL && !id_wellformed(id)) {
error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "id", "an identifier");
error_append_hint(errp, "Identifiers consist of letters, digits, "
@@ -785,7 +791,10 @@ object_new_with_props_helper(const char *typename,
}
if (id != NULL) {
-object_property_add_child(parent, id, obj);
+object_property_try_add_child(parent, id, obj, errp);
+if (*errp) {
+goto error;
+}
}
uc = (UserCreatable *)object_dynamic_cast(obj, TYPE_USER_CREATABLE);
@@ -798,7 +807,6 @@ object_new_with_props_helper(const char *typename,
}
}
