[PATCH-for-10.1 0/6] qom: Add object_class_implements_type()

[PATCH-for-10.1 0/6] qom: Add object_class_implements_type()

[Philippe Mathieu-Daudé]

Hi,

object_class_dynamic_cast() checks whether a class implements
a type name, and return the class casted appropriately. This
also works with interfaces, except when an interface is
implemented multiple times (by intermediate abstract parents /
interfaces).

This series factors object_class_implements_type() out of
object_class_dynamic_cast() and use it (at least the meaning
seems clearer to me when reviewing).

I could get it working with object_class_foreach() but for
some reason fail at writing a proper test. Posting the last
patch as RFC so we can discuss it on the list.

Regards,

Phil.

Philippe Mathieu-Daudé (6):
  qom: Factor object_class_dynamic_cast_ambiguous() out
  qom: Add object_class_implements_type()
  qom: Test object_class_implements_type()
  qom: Prefer object_class_implements_type() to check type
    implementation
  qom: Use object_class_implements_type() in object_class_foreach()
  XXX qom: Test object_class_get_list()

 include/qom/object.h             | 10 ++++++
 chardev/char.c                   |  2 +-
 hw/arm/xlnx-versal-virt.c        |  2 +-
 hw/core/cpu-common.c             |  2 +-
 hw/core/machine.c                |  4 +--
 hw/i386/xen/xen_platform.c       |  2 +-
 hw/pci/pci.c                     |  6 ++--
 qom/object.c                     | 29 ++++++++++++++---
 qom/object_interfaces.c          |  2 +-
 qom/qom-qmp-cmds.c               |  4 +--
 system/qdev-monitor.c            |  4 +--
 system/tpm.c                     |  2 +-
 target/alpha/cpu.c               |  2 +-
 target/rx/cpu.c                  |  2 +-
 tests/unit/check-qom-interface.c | 56 ++++++++++++++++++++++++++++++++
 15 files changed, 108 insertions(+), 21 deletions(-)

-- 
2.47.1

[PATCH-for-10.1 1/6] qom: Factor object_class_dynamic_cast_ambiguous() out

[Philippe Mathieu-Daudé]

In order to allow matching ambiguous types in the next commit,
factor object_class_dynamic_cast_ambiguous() out of
object_class_dynamic_cast().

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 qom/object.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/qom/object.c b/qom/object.c
index 01618d06bd8..1620a87ef44 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -940,8 +940,9 @@ out:
     return obj;
 }
 
-ObjectClass *object_class_dynamic_cast(ObjectClass *class,
-                                       const char *typename)
+static ObjectClass *object_class_dynamic_cast_ambiguous(ObjectClass *class,
+                                                        const char *typename,
+                                                        bool *ambiguous)
 {
     ObjectClass *ret = NULL;
     TypeImpl *target_type;
@@ -977,6 +978,9 @@ ObjectClass *object_class_dynamic_cast(ObjectClass *class,
             }
          }
 
+        if (ambiguous) {
+            *ambiguous = found > 0;
+        }
         /* The match was ambiguous, don't allow a cast */
         if (found > 1) {
             ret = NULL;
@@ -988,6 +992,12 @@ ObjectClass *object_class_dynamic_cast(ObjectClass *class,
     return ret;
 }
 
+ObjectClass *object_class_dynamic_cast(ObjectClass *class,
+                                       const char *typename)
+{
+    return object_class_dynamic_cast_ambiguous(class, typename, NULL);
+}
+
 ObjectClass *object_class_dynamic_cast_assert(ObjectClass *class,
                                               const char *typename,
                                               const char *file, int line,
-- 
2.47.1

[PATCH-for-10.1 2/6] qom: Add object_class_implements_type()

[Philippe Mathieu-Daudé]

Add the object_class_implements_type() method to check whether
a class implement a type, which can be ambiguous for interfaces.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/qom/object.h | 10 ++++++++++
 qom/object.c         | 11 +++++++++++
 2 files changed, 21 insertions(+)

diff --git a/include/qom/object.h b/include/qom/object.h
index 9192265db76..fd74d4be89f 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -995,6 +995,16 @@ const char *object_class_get_name(ObjectClass *klass);
  */
 bool object_class_is_abstract(ObjectClass *klass);
 
+/**
+ * object_class_implements_type:
+ * @klass: The class to check the implementation for.
+ * @typename: The QOM typename of the implementation to check to.
+ *
+ * Returns: %true if @klass implements %typename, %false otherwise.
+ */
+bool object_class_implements_type(ObjectClass *class,
+                                  const char *typename);
+
 /**
  * object_class_by_name:
  * @typename: The QOM typename to obtain the class for.
diff --git a/qom/object.c b/qom/object.c
index 1620a87ef44..b9948937935 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -992,6 +992,17 @@ static ObjectClass *object_class_dynamic_cast_ambiguous(ObjectClass *class,
     return ret;
 }
 
+bool object_class_implements_type(ObjectClass *class,
+                                  const char *typename)
+{
+    ObjectClass *k;
+    bool ambiguous = false;
+
+    k = object_class_dynamic_cast_ambiguous(class, typename, &ambiguous);
+
+    return k || ambiguous;
+}
+
 ObjectClass *object_class_dynamic_cast(ObjectClass *class,
                                        const char *typename)
 {
-- 
2.47.1

[PATCH-for-10.1 3/6] qom: Test object_class_implements_type()

[Philippe Mathieu-Daudé]

Test TYPE_INDIRECT_IMPL (indirectly) implements
TYPE_TEST_IF (via TYPE_TEST_IF2A or TYPE_TEST_IF2B
interface).

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 tests/unit/check-qom-interface.c | 40 ++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/tests/unit/check-qom-interface.c b/tests/unit/check-qom-interface.c
index c99be97ed8b..c43a63e8b3c 100644
--- a/tests/unit/check-qom-interface.c
+++ b/tests/unit/check-qom-interface.c
@@ -36,6 +36,19 @@ static const TypeInfo test_if_info = {
     .class_size = sizeof(TestIfClass),
 };
 
+#define TYPE_TEST_IF2A "test-interface-level2a"
+#define TYPE_TEST_IF2B "test-interface-level2b"
+
+static const TypeInfo test_if2a_info = {
+    .name          = TYPE_TEST_IF2A,
+    .parent        = TYPE_TEST_IF,
+};
+
+static const TypeInfo test_if2b_info = {
+    .name          = TYPE_TEST_IF2B,
+    .parent        = TYPE_TEST_IF,
+};
+
 #define PATTERN 0xFAFBFCFD
 
 static void test_class_init(ObjectClass *oc, void *data)
@@ -58,6 +71,18 @@ static const TypeInfo direct_impl_info = {
     }
 };
 
+#define TYPE_INDIRECT_IMPL "indirect-impl"
+
+static const TypeInfo indirect_impl_info = {
+    .name = TYPE_INDIRECT_IMPL,
+    .parent = TYPE_DIRECT_IMPL,
+    .interfaces = (InterfaceInfo[]) {
+        { TYPE_TEST_IF2A },
+        { TYPE_TEST_IF2B },
+        { }
+    }
+};
+
 #define TYPE_INTERMEDIATE_IMPL "intermediate-impl"
 
 static const TypeInfo intermediate_impl_info = {
@@ -86,18 +111,33 @@ static void interface_intermediate_test(void)
     test_interface_impl(TYPE_INTERMEDIATE_IMPL);
 }
 
+static void interface_ambiguous_test(void)
+{
+    Object *obj = object_new(TYPE_INDIRECT_IMPL);
+    ObjectClass *klass = object_get_class(obj);
+
+    g_assert(object_class_implements_type(klass, TYPE_TEST_IF2A));
+    g_assert(object_class_implements_type(klass, TYPE_TEST_IF2B));
+    g_assert(object_class_implements_type(klass, TYPE_TEST_IF));
+    object_unref(obj);
+}
+
 int main(int argc, char **argv)
 {
     g_test_init(&argc, &argv, NULL);
 
     module_call_init(MODULE_INIT_QOM);
     type_register_static(&test_if_info);
+    type_register_static(&test_if2a_info);
+    type_register_static(&test_if2b_info);
     type_register_static(&direct_impl_info);
+    type_register_static(&indirect_impl_info);
     type_register_static(&intermediate_impl_info);
 
     g_test_add_func("/qom/interface/direct_impl", interface_direct_test);
     g_test_add_func("/qom/interface/intermediate_impl",
                     interface_intermediate_test);
+    g_test_add_func("/qom/interface/ambiguous_impl", interface_ambiguous_test);
 
     return g_test_run();
 }
-- 
2.47.1

[PATCH-for-10.1 4/6] qom: Prefer object_class_implements_type() to check type implementation

[Philippe Mathieu-Daudé]

When we only want to check a type is implemented and
are not interested by the returned class, prefer
object_class_implements_type() over object_class_dynamic_cast().

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 chardev/char.c             | 2 +-
 hw/arm/xlnx-versal-virt.c  | 2 +-
 hw/core/cpu-common.c       | 2 +-
 hw/core/machine.c          | 4 ++--
 hw/i386/xen/xen_platform.c | 2 +-
 hw/pci/pci.c               | 6 +++---
 qom/object.c               | 2 +-
 qom/object_interfaces.c    | 2 +-
 qom/qom-qmp-cmds.c         | 4 ++--
 system/qdev-monitor.c      | 4 ++--
 system/tpm.c               | 2 +-
 target/alpha/cpu.c         | 2 +-
 target/rx/cpu.c            | 2 +-
 13 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/chardev/char.c b/chardev/char.c
index 5a9e9762adc..b7125e3aebe 100644
--- a/chardev/char.c
+++ b/chardev/char.c
@@ -528,7 +528,7 @@ static const ChardevClass *char_get_class(const char *driver, Error **errp)
     oc = module_object_class_by_name(typename);
     g_free(typename);
 
-    if (!object_class_dynamic_cast(oc, TYPE_CHARDEV)) {
+    if (!object_class_implements_type(oc, TYPE_CHARDEV)) {
         error_setg(errp, "'%s' is not a valid char driver name", driver);
         return NULL;
     }
diff --git a/hw/arm/xlnx-versal-virt.c b/hw/arm/xlnx-versal-virt.c
index 0c6f0359e3d..9313780a4c2 100644
--- a/hw/arm/xlnx-versal-virt.c
+++ b/hw/arm/xlnx-versal-virt.c
@@ -760,7 +760,7 @@ static void versal_virt_init(MachineState *machine)
             flash_klass = object_class_by_name(s->ospi_model);
             if (!flash_klass ||
                 object_class_is_abstract(flash_klass) ||
-                !object_class_dynamic_cast(flash_klass, TYPE_M25P80)) {
+                !object_class_implements_type(flash_klass, TYPE_M25P80)) {
                 error_report("'%s' is either abstract or"
                        " not a subtype of m25p80", s->ospi_model);
                 exit(1);
diff --git a/hw/core/cpu-common.c b/hw/core/cpu-common.c
index 9064dd24f82..89c2dd53599 100644
--- a/hw/core/cpu-common.c
+++ b/hw/core/cpu-common.c
@@ -144,7 +144,7 @@ ObjectClass *cpu_class_by_name(const char *typename, const char *cpu_model)
     assert(cc->class_by_name);
     assert(cpu_model);
     oc = cc->class_by_name(cpu_model);
-    if (object_class_dynamic_cast(oc, typename) &&
+    if (object_class_implements_type(oc, typename) &&
         !object_class_is_abstract(oc)) {
         return oc;
     }
diff --git a/hw/core/machine.c b/hw/core/machine.c
index f52a4f2273b..712d07e1593 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -735,7 +735,7 @@ bool device_type_is_dynamic_sysbus(MachineClass *mc, const char *type)
     for (wl = mc->allowed_dynamic_sysbus_devices;
          !allowed && wl;
          wl = wl->next) {
-        allowed |= !!object_class_dynamic_cast(klass, wl->value);
+        allowed |= object_class_implements_type(klass, wl->value);
     }
 
     return allowed;
@@ -1553,7 +1553,7 @@ static bool is_cpu_type_supported(const MachineState *machine, Error **errp)
     if (mc->valid_cpu_types) {
         assert(mc->valid_cpu_types[0] != NULL);
         for (i = 0; mc->valid_cpu_types[i]; i++) {
-            if (object_class_dynamic_cast(oc, mc->valid_cpu_types[i])) {
+            if (object_class_implements_type(oc, mc->valid_cpu_types[i])) {
                 break;
             }
         }
diff --git a/hw/i386/xen/xen_platform.c b/hw/i386/xen/xen_platform.c
index dd648a2ee94..761357657f1 100644
--- a/hw/i386/xen/xen_platform.c
+++ b/hw/i386/xen/xen_platform.c
@@ -144,7 +144,7 @@ static void del_nic_peer(NICState *nic, void *opaque)
     ObjectClass *klass = module_object_class_by_name(nc->model);
 
     /* Only delete peers of PCI NICs that we're about to delete */
-    if (!klass || !object_class_dynamic_cast(klass, TYPE_PCI_DEVICE)) {
+    if (!klass || !object_class_implements_type(klass, TYPE_PCI_DEVICE)) {
         return;
     }
 
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 2844ec55560..ff2373b9876 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -2244,12 +2244,12 @@ static void pci_qdev_realize(DeviceState *qdev, Error **errp)
     /* initialize cap_present for pci_is_express() and pci_config_size(),
      * Note that hybrid PCIs are not set automatically and need to manage
      * QEMU_PCI_CAP_EXPRESS manually */
-    if (object_class_dynamic_cast(klass, INTERFACE_PCIE_DEVICE) &&
-       !object_class_dynamic_cast(klass, INTERFACE_CONVENTIONAL_PCI_DEVICE)) {
+    if (object_class_implements_type(klass, INTERFACE_PCIE_DEVICE) &&
+       !object_class_implements_type(klass, INTERFACE_CONVENTIONAL_PCI_DEVICE)) {
         pci_dev->cap_present |= QEMU_PCI_CAP_EXPRESS;
     }
 
-    if (object_class_dynamic_cast(klass, INTERFACE_CXL_DEVICE)) {
+    if (object_class_implements_type(klass, INTERFACE_CXL_DEVICE)) {
         pci_dev->cap_present |= QEMU_PCIE_CAP_CXL;
     }
 
diff --git a/qom/object.c b/qom/object.c
index b9948937935..536308c0553 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -894,7 +894,7 @@ bool object_set_propv(Object *obj,
 
 Object *object_dynamic_cast(Object *obj, const char *typename)
 {
-    if (obj && object_class_dynamic_cast(object_get_class(obj), typename)) {
+    if (obj && object_class_implements_type(object_get_class(obj), typename)) {
         return obj;
     }
 
diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c
index 1ffea1a7288..333c5f94e19 100644
--- a/qom/object_interfaces.c
+++ b/qom/object_interfaces.c
@@ -98,7 +98,7 @@ Object *user_creatable_add_type(const char *type, const char *id,
         return NULL;
     }
 
-    if (!object_class_dynamic_cast(klass, TYPE_USER_CREATABLE)) {
+    if (!object_class_implements_type(klass, TYPE_USER_CREATABLE)) {
         error_setg(errp, "object type '%s' isn't supported by object-add",
                    type);
         return NULL;
diff --git a/qom/qom-qmp-cmds.c b/qom/qom-qmp-cmds.c
index e8665476182..1a1f0e1dfd6 100644
--- a/qom/qom-qmp-cmds.c
+++ b/qom/qom-qmp-cmds.c
@@ -134,7 +134,7 @@ ObjectPropertyInfoList *qmp_device_list_properties(const char *typename,
         return NULL;
     }
 
-    if (!object_class_dynamic_cast(klass, TYPE_DEVICE)
+    if (!object_class_implements_type(klass, TYPE_DEVICE)
         || object_class_is_abstract(klass)) {
         error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "typename",
                    "a non-abstract device type");
@@ -193,7 +193,7 @@ ObjectPropertyInfoList *qmp_qom_list_properties(const char *typename,
         return NULL;
     }
 
-    if (!object_class_dynamic_cast(klass, TYPE_OBJECT)) {
+    if (!object_class_implements_type(klass, TYPE_OBJECT)) {
         error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "typename",
                    "a QOM type");
         return NULL;
diff --git a/system/qdev-monitor.c b/system/qdev-monitor.c
index 5588ed2047d..7e4cfe0f0e5 100644
--- a/system/qdev-monitor.c
+++ b/system/qdev-monitor.c
@@ -246,7 +246,7 @@ static DeviceClass *qdev_get_device_class(const char **driver, Error **errp)
         }
     }
 
-    if (!object_class_dynamic_cast(oc, TYPE_DEVICE)) {
+    if (!object_class_implements_type(oc, TYPE_DEVICE)) {
         if (*driver != original_name) {
             error_setg(errp, "'%s' (alias '%s') is not a valid device model"
                        " name", original_name, *driver);
@@ -269,7 +269,7 @@ static DeviceClass *qdev_get_device_class(const char **driver, Error **errp)
         return NULL;
     }
 
-    if (object_class_dynamic_cast(oc, TYPE_SYS_BUS_DEVICE)) {
+    if (object_class_implements_type(oc, TYPE_SYS_BUS_DEVICE)) {
         /* sysbus devices need to be allowed by the machine */
         MachineClass *mc = MACHINE_CLASS(object_get_class(qdev_get_machine()));
         if (!device_type_is_dynamic_sysbus(mc, *driver)) {
diff --git a/system/tpm.c b/system/tpm.c
index 8df0f6e72b3..eeb1bd0e27c 100644
--- a/system/tpm.c
+++ b/system/tpm.c
@@ -34,7 +34,7 @@ tpm_be_find_by_type(enum TpmType type)
     oc = object_class_by_name(typename);
     g_free(typename);
 
-    if (!object_class_dynamic_cast(oc, TYPE_TPM_BACKEND)) {
+    if (!object_class_implements_type(oc, TYPE_TPM_BACKEND)) {
         return NULL;
     }
 
diff --git a/target/alpha/cpu.c b/target/alpha/cpu.c
index 584c2aa76bd..f9ba199e794 100644
--- a/target/alpha/cpu.c
+++ b/target/alpha/cpu.c
@@ -136,7 +136,7 @@ static ObjectClass *alpha_cpu_class_by_name(const char *cpu_model)
     int i;
 
     oc = object_class_by_name(cpu_model);
-    if (oc != NULL && object_class_dynamic_cast(oc, TYPE_ALPHA_CPU) != NULL) {
+    if (oc != NULL && object_class_implements_type(oc, TYPE_ALPHA_CPU)) {
         return oc;
     }
 
diff --git a/target/rx/cpu.c b/target/rx/cpu.c
index 0ba0d55ab5b..2cc4e1df7aa 100644
--- a/target/rx/cpu.c
+++ b/target/rx/cpu.c
@@ -119,7 +119,7 @@ static ObjectClass *rx_cpu_class_by_name(const char *cpu_model)
     char *typename;
 
     oc = object_class_by_name(cpu_model);
-    if (oc != NULL && object_class_dynamic_cast(oc, TYPE_RX_CPU) != NULL) {
+    if (oc != NULL && object_class_implements_type(oc, TYPE_RX_CPU)) {
         return oc;
     }
     typename = g_strdup_printf(RX_CPU_TYPE_NAME("%s"), cpu_model);
-- 
2.47.1

[PATCH-for-10.1 5/6] qom: Use object_class_implements_type() in object_class_foreach()

[Philippe Mathieu-Daudé]

Previously classes indirectly implementing an interface more than
once were triggering the ambiguous cast assertion. Since we are not
interested in the returned class, but only whether it implements or
not, use object_class_implements_type().

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 qom/object.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qom/object.c b/qom/object.c
index 536308c0553..6733adcd6b2 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -1136,7 +1136,7 @@ static void object_class_foreach_tramp(gpointer key, gpointer value,
     }
 
     if (data->implements_type && 
-        !object_class_dynamic_cast(k, data->implements_type)) {
+        !object_class_implements_type(k, data->implements_type)) {
         return;
     }
 
-- 
2.47.1

[RFC PATCH-for-10.1 6/6] qom: Test object_class_get_list()

[Philippe Mathieu-Daudé]

RFC FIXME: this doesn't work well :/

In the first iteration the TYPE_TEST_IF is ambiguous (matched 2 times)
so OBJECT_CLASS_CHECK() triggers the cast exception.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 tests/unit/check-qom-interface.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/tests/unit/check-qom-interface.c b/tests/unit/check-qom-interface.c
index c43a63e8b3c..8a997220e06 100644
--- a/tests/unit/check-qom-interface.c
+++ b/tests/unit/check-qom-interface.c
@@ -115,10 +115,26 @@ static void interface_ambiguous_test(void)
 {
     Object *obj = object_new(TYPE_INDIRECT_IMPL);
     ObjectClass *klass = object_get_class(obj);
+    GSList *list, *el;
 
     g_assert(object_class_implements_type(klass, TYPE_TEST_IF2A));
     g_assert(object_class_implements_type(klass, TYPE_TEST_IF2B));
     g_assert(object_class_implements_type(klass, TYPE_TEST_IF));
+    list = object_class_get_list(TYPE_TEST_IF, true);
+    for (el = list; el; el = el->next) {
+        TestIfClass *ioc = el->data;
+        printf("%x %x\n", ioc->test, PATTERN);
+        //g_assert(ioc->test == PATTERN);
+    }
+    g_free(list);
+    list = object_class_get_list(TYPE_TEST_IF2A, true);
+    for (el = list; el; el = el->next) {
+        TestIfClass *dc = OBJECT_CLASS_CHECK(TestIfClass, el->data,
+                                             TYPE_TEST_IF2A);
+        printf("%x %x\n", dc->test, PATTERN);
+        //g_assert(ioc->test == PATTERN);
+    }
+    g_free(list);
     object_unref(obj);
 }
 
-- 
2.47.1

Re: [PATCH-for-10.1 0/6] qom: Add object_class_implements_type()

[Philippe Mathieu-Daudé]

On 20/3/25 16:47, Philippe Mathieu-Daudé wrote:
> Hi,
> 
> object_class_dynamic_cast() checks whether a class implements
> a type name, and return the class casted appropriately. This
> also works with interfaces, except when an interface is
> implemented multiple times (by intermediate abstract parents /
> interfaces).

I had a chat with Pierrick where he said diamond graph often leads
to problems, so it is safer to not allow multiple inheritance that
way; and returning the casted class is simple enough.

I'll see how to do differently, effectively not pursuing this series.

> 
> This series factors object_class_implements_type() out of
> object_class_dynamic_cast() and use it (at least the meaning
> seems clearer to me when reviewing).
> 
> I could get it working with object_class_foreach() but for
> some reason fail at writing a proper test. Posting the last
> patch as RFC so we can discuss it on the list.
> 
> Regards,
> 
> Phil.