@samitouri / QOSamiQemu / commits / 4d6a50a96e

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é <philmd@linaro.org> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>

Daniel P. Berrangé committed Apr 17, 2026 at 15:27 UTC 4d6a50a96e27fb9cf7d377df231f90c80cabaef5

3 files changed +187 -22

include/qom/object.h

+47

@@ -719,6 +719,53 @@ Object *object_new_with_props_from_qdict(const char *typename,

719 Visitor *v,

720 Error **errp);

721

722 +/**

723 + * object_new_with_props_parentless:

724 + * @typename: The name of the type of the object to instantiate.

725 + * @errp: pointer to error object

726 + * @...: list of property names and values

727 + *

728 + * Behaviour as object_new_with_props(), except the object

729 + * will not be added to any parent and thus the caller will

730 + * own the returned instance. The caller must call

731 + * object_unref when it is no longer required.

732 + */

733 +Object *object_new_with_props_parentless(const char *typename,

734 + Error **errp,

735 + ...) G_GNUC_NULL_TERMINATED;

736 +

737 +/**

738 + * object_new_with_propv_parentless:

739 + * @typename: The name of the type of the object to instantiate.

740 + * @vargs: list of property names and values

741 + * @errp: pointer to error object

742 + *

743 + * Behaviour as object_new_with_propv(), except the object

744 + * will not be added to any parent and thus the caller will

745 + * own the returned instance. The caller must call

746 + * object_unref when it is no longer required.

747 + */

748 +Object *object_new_with_propv_parentless(const char *typename,

749 + va_list vargs,

750 + Error **errp);

751 +

752 +/**

753 + * object_new_with_props_from_qdict_parentless:

754 + * @typename: The name of the type of the object to instantiate.

755 + * @props: dictionary of property names and values

756 + * @v: visitor to iterate over @props

757 + * @errp: pointer to error object

758 + *

759 + * Behaviour as object_new_with_props_from_qdict(), except the

760 + * object will not be added to any parent and thus the caller

761 + * will own the returned instance. The caller must call

762 + * object_unref when it is no longer required.

763 + */

764 +Object *object_new_with_props_from_qdict_parentless(const char *typename,

765 + const QDict *props,

766 + Visitor *v,

767 + Error **errp);

768 +

769 /**

770 * object_set_props:

771 * @obj: the object instance to set properties on

qom/object.c

+65 -9

@@ -740,6 +740,8 @@ Object *object_new_with_props(const char *typename,

740 va_list vargs;

741 Object *obj;

742

743 + assert(parent != NULL);

744 + assert(id != NULL);

745 va_start(vargs, errp);

746 obj = object_new_with_propv(typename, parent, id, vargs, errp);

747 va_end(vargs);

@@ -757,10 +759,14 @@ object_new_with_props_helper(const char *typename,

759 Error **errp),

760 Error **errp)

761 {

762 + ERRP_GUARD();

763 Object *obj;

764 ObjectClass *klass;

765 UserCreatable *uc;

766

767 + assert((id != NULL && parent != NULL) ||

768 + (id == NULL && parent == NULL));

769 +

770 if (id != NULL && !id_wellformed(id)) {

771 error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "id", "an identifier");

772 error_append_hint(errp, "Identifiers consist of letters, digits, "

@@ -785,7 +791,10 @@ object_new_with_props_helper(const char *typename,

791 }

792

793 if (id != NULL) {

788 - object_property_add_child(parent, id, obj);

794 + object_property_try_add_child(parent, id, obj, errp);

795 + if (*errp) {

796 + goto error;

797 + }

798 }

799

800 uc = (UserCreatable *)object_dynamic_cast(obj, TYPE_USER_CREATABLE);

@@ -798,7 +807,6 @@ object_new_with_props_helper(const char *typename,

807 }

808 }

809

801 - object_unref(obj);

810 return obj;

811

812 error:

@@ -826,7 +834,8 @@ Object *object_new_with_propv(const char *typename,

834 {

835 Object *obj;

836 struct ObjectNewVargsData data;

829 -

837 + assert(parent != NULL);

838 + assert(id != NULL);

839 va_copy(data.vargs, vargs);

840 obj = object_new_with_props_helper(typename,

841 parent,

@@ -835,6 +844,9 @@ Object *object_new_with_propv(const char *typename,

844 object_new_with_propv_setter,

845 errp);

846 va_end(data.vargs);

847 + if (obj) {

848 + object_unref(obj);

849 + }

850 return obj;

851 }

852

@@ -859,12 +871,56 @@ Object *object_new_with_props_from_qdict(const char *typename,

871 Error **errp)

872 {

873 struct ObjectNewQDictData data = { props, v };

862 - return object_new_with_props_helper(typename,

863 - parent,

864 - id,

865 - &data,

866 - object_new_with_qdict_setter,

867 - errp);

874 + Object *obj;

875 + assert(parent != NULL);

876 + assert(id != NULL);

877 + obj = object_new_with_props_helper(typename,

878 + parent,

879 + id,

880 + &data,

881 + object_new_with_qdict_setter,

882 + errp);

883 + if (obj) {

884 + object_unref(obj);

885 + }

886 + return obj;

887 +}

888 +

889 +Object *object_new_with_props_parentless(const char *typename,

890 + Error **errp,

891 + ...)

892 +{

893 + va_list vargs;

894 + Object *obj;

895 +

896 + va_start(vargs, errp);

897 + obj = object_new_with_propv_parentless(typename, vargs, errp);

898 + va_end(vargs);

899 +

900 + return obj;

901 +}

902 +

903 +Object *object_new_with_propv_parentless(const char *typename,

904 + va_list vargs,

905 + Error **errp)

906 +{

907 + Object *ret;

908 + struct ObjectNewVargsData data;

909 + va_copy(data.vargs, vargs);

910 + ret = object_new_with_props_helper(typename, NULL, NULL, &data,

911 + object_new_with_propv_setter, errp);

912 + va_end(data.vargs);

913 + return ret;

914 +}

915 +

916 +Object *object_new_with_props_from_qdict_parentless(const char *typename,

917 + const QDict *props,

918 + Visitor *v,

919 + Error **errp)

920 +{

921 + struct ObjectNewQDictData data = { props, v };

922 + return object_new_with_props_helper(typename, NULL, NULL, &data,

923 + object_new_with_qdict_setter, errp);

924 }

925

926 bool object_set_props(Object *obj,

tests/unit/check-qom-proplist.c

+75 -13

@@ -336,7 +336,7 @@ static QemuOptsList qemu_object_opts = {

336 };

337

338

339 -static void test_dummy_createv(void)

339 +static void test_dummy_createv_tree(void)

340 {

341 Error *err = NULL;

342 Object *parent = object_get_objects_root();

@@ -351,6 +351,7 @@ static void test_dummy_createv(void)

351 NULL));

352

353 g_assert(err == NULL);

354 + g_assert_cmpint(dobj->parent_obj.ref, ==, 1);

355 g_assert_cmpstr(dobj->sv, ==, "Hiss hiss hiss");

356 g_assert(dobj->bv == true);

357 g_assert(dobj->av == DUMMY_PLATYPUS);

@@ -362,9 +363,30 @@ static void test_dummy_createv(void)

363 }

364

365

365 -static Object *new_helper(Error **errp,

366 - Object *parent,

367 - ...)

366 +static void test_dummy_createv_parentless(void)

367 +{

368 + Error *err = NULL;

369 + DummyObject *dobj = DUMMY_OBJECT(

370 + object_new_with_props_parentless(TYPE_DUMMY,

371 + &err,

372 + "bv", "yes",

373 + "sv", "Hiss hiss hiss",

374 + "av", "platypus",

375 + NULL));

376 +

377 + g_assert(err == NULL);

378 + g_assert_cmpint(dobj->parent_obj.ref, ==, 1);

379 + g_assert_cmpstr(dobj->sv, ==, "Hiss hiss hiss");

380 + g_assert(dobj->bv == true);

381 + g_assert(dobj->av == DUMMY_PLATYPUS);

382 +

383 + object_unref(OBJECT(dobj));

384 +}

385 +

386 +

387 +static Object *new_helper_tree(Error **errp,

388 + Object *parent,

389 + ...)

390 {

391 va_list vargs;

392 Object *obj;

@@ -379,19 +401,20 @@ static Object *new_helper(Error **errp,

401 return obj;

402 }

403

382 -static void test_dummy_createlist(void)

404 +static void test_dummy_createlist_tree(void)

405 {

406 Error *err = NULL;

407 Object *parent = object_get_objects_root();

408 DummyObject *dobj = DUMMY_OBJECT(

387 - new_helper(&err,

388 - parent,

389 - "bv", "yes",

390 - "sv", "Hiss hiss hiss",

391 - "av", "platypus",

392 - NULL));

409 + new_helper_tree(&err,

410 + parent,

411 + "bv", "yes",

412 + "sv", "Hiss hiss hiss",

413 + "av", "platypus",

414 + NULL));

415

416 g_assert(err == NULL);

417 + g_assert_cmpint(dobj->parent_obj.ref, ==, 1);

418 g_assert_cmpstr(dobj->sv, ==, "Hiss hiss hiss");

419 g_assert(dobj->bv == true);

420 g_assert(dobj->av == DUMMY_PLATYPUS);

@@ -402,6 +425,39 @@ static void test_dummy_createlist(void)

425 object_unparent(OBJECT(dobj));

426 }

427

428 +static Object *new_helper_parentless(Error **errp,

429 + ...)

430 +{

431 + va_list vargs;

432 + Object *obj;

433 +

434 + va_start(vargs, errp);

435 + obj = object_new_with_propv_parentless(TYPE_DUMMY,

436 + vargs,

437 + errp);

438 + va_end(vargs);

439 + return obj;

440 +}

441 +

442 +static void test_dummy_createlist_parentless(void)

443 +{

444 + Error *err = NULL;

445 + DummyObject *dobj = DUMMY_OBJECT(

446 + new_helper_parentless(&err,

447 + "bv", "yes",

448 + "sv", "Hiss hiss hiss",

449 + "av", "platypus",

450 + NULL));

451 +

452 + g_assert(err == NULL);

453 + g_assert_cmpint(dobj->parent_obj.ref, ==, 1);

454 + g_assert_cmpstr(dobj->sv, ==, "Hiss hiss hiss");

455 + g_assert(dobj->bv == true);

456 + g_assert(dobj->av == DUMMY_PLATYPUS);

457 +

458 + object_unref(OBJECT(dobj));

459 +}

460 +

461 static bool test_create_obj(QDict *qdict, Error **errp)

462 {

463 Visitor *v = qobject_input_visitor_new_keyval(QOBJECT(qdict));

@@ -658,8 +714,14 @@ int main(int argc, char **argv)

714 type_register_static(&dummy_bus_info);

715 type_register_static(&dummy_backend_info);

716

661 - g_test_add_func("/qom/proplist/createlist", test_dummy_createlist);

662 - g_test_add_func("/qom/proplist/createv", test_dummy_createv);

717 + g_test_add_func("/qom/proplist/createlist/tree",

718 + test_dummy_createlist_tree);

719 + g_test_add_func("/qom/proplist/createlist/parentless",

720 + test_dummy_createlist_parentless);

721 + g_test_add_func("/qom/proplist/createv/tree",

722 + test_dummy_createv_tree);

723 + g_test_add_func("/qom/proplist/createv/parentless",

724 + test_dummy_createv_parentless);

725 g_test_add_func("/qom/proplist/createcmdline", test_dummy_createcmdl);

726 g_test_add_func("/qom/proplist/badenum", test_dummy_badenum);

727 g_test_add_func("/qom/proplist/getenum", test_dummy_getenum);