qemu-riscv.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Markus Armbruster <armbru@redhat.com>
To: qemu-devel@nongnu.org
Cc: peter.maydell@linaro.org, pbonzini@redhat.com,
	gaosong@loongson.cn, palmer@dabbelt.com,
	alistair.francis@wdc.com, bin.meng@windriver.com,
	liwei1518@gmail.com, dbarboza@ventanamicro.com,
	zhiwei_liu@linux.alibaba.com, richard.henderson@linaro.org,
	david@redhat.com, iii@linux.ibm.com, thuth@redhat.com,
	qemu-arm@nongnu.org, qemu-riscv@nongnu.org,
	qemu-s390x@nongnu.org
Subject: [PATCH 1/4] target: Simplify type checks for CpuModelInfo member @props
Date: Tue,  5 Mar 2024 15:59:15 +0100	[thread overview]
Message-ID: <20240305145919.2186971-2-armbru@redhat.com> (raw)
In-Reply-To: <20240305145919.2186971-1-armbru@redhat.com>

CpuModelInfo member @props is semantically a mapping from name to
value, and syntactically a JSON object on the wire.  This translates
to QDict in C.  Since the QAPI schema language lacks the means to
express 'object', we use 'any' instead.  This is QObject in C.
Commands taking a CpuModelInfo argument need to check the QObject is a
QDict.

For arm, riscv, and s390x, the code checks right before passing the
QObject to visit_start_struct().  visit_start_struct() then checks
again.

Delete the first check.

The error message for @props that are not an object changes slightly
to the the message we get for this kind of type error in other
contexts.  Minor improvement.

Additionally, error messages about members of @props now refer to
'props.prop-name' instead of just 'prop-name'.  Another minor
improvement.

Both changes are visible in tests/qtest/arm-cpu-features.c.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 target/arm/arm-qmp-cmds.c        | 15 ++++-----------
 target/riscv/riscv-qmp-cmds.c    | 19 +++++--------------
 target/s390x/cpu_models_sysemu.c | 15 ++++-----------
 tests/qtest/arm-cpu-features.c   | 12 ++++++------
 4 files changed, 19 insertions(+), 42 deletions(-)

diff --git a/target/arm/arm-qmp-cmds.c b/target/arm/arm-qmp-cmds.c
index 2250cd7ddf..0167759730 100644
--- a/target/arm/arm-qmp-cmds.c
+++ b/target/arm/arm-qmp-cmds.c
@@ -104,7 +104,7 @@ CpuModelExpansionInfo *qmp_query_cpu_model_expansion(CpuModelExpansionType type,
                                                      Error **errp)
 {
     CpuModelExpansionInfo *expansion_info;
-    const QDict *qdict_in = NULL;
+    const QDict *qdict_in;
     QDict *qdict_out;
     ObjectClass *oc;
     Object *obj;
@@ -151,27 +151,20 @@ CpuModelExpansionInfo *qmp_query_cpu_model_expansion(CpuModelExpansionType type,
         }
     }
 
-    if (model->props) {
-        qdict_in = qobject_to(QDict, model->props);
-        if (!qdict_in) {
-            error_setg(errp, QERR_INVALID_PARAMETER_TYPE, "props", "dict");
-            return NULL;
-        }
-    }
-
     obj = object_new(object_class_get_name(oc));
 
-    if (qdict_in) {
+    if (model->props) {
         Visitor *visitor;
         Error *err = NULL;
 
         visitor = qobject_input_visitor_new(model->props);
-        if (!visit_start_struct(visitor, NULL, NULL, 0, errp)) {
+        if (!visit_start_struct(visitor, "props", NULL, 0, errp)) {
             visit_free(visitor);
             object_unref(obj);
             return NULL;
         }
 
+        qdict_in = qobject_to(QDict, model->props);
         i = 0;
         while ((name = cpu_model_advertised_features[i++]) != NULL) {
             if (qdict_get(qdict_in, name)) {
diff --git a/target/riscv/riscv-qmp-cmds.c b/target/riscv/riscv-qmp-cmds.c
index c48b9cfa67..69dde0c3e7 100644
--- a/target/riscv/riscv-qmp-cmds.c
+++ b/target/riscv/riscv-qmp-cmds.c
@@ -129,18 +129,19 @@ static void riscv_obj_add_profiles_qdict(Object *obj, QDict *qdict_out)
 }
 
 static void riscv_cpuobj_validate_qdict_in(Object *obj, QObject *props,
-                                           const QDict *qdict_in,
                                            Error **errp)
 {
+    const QDict *qdict_in;
     const QDictEntry *qe;
     Visitor *visitor;
     Error *local_err = NULL;
 
     visitor = qobject_input_visitor_new(props);
-    if (!visit_start_struct(visitor, NULL, NULL, 0, &local_err)) {
+    if (!visit_start_struct(visitor, "props", NULL, 0, &local_err)) {
         goto err;
     }
 
+    qdict_in = qobject_to(QDict, props);
     for (qe = qdict_first(qdict_in); qe; qe = qdict_next(qdict_in, qe)) {
         object_property_find_err(obj, qe->key, &local_err);
         if (local_err) {
@@ -170,7 +171,6 @@ CpuModelExpansionInfo *qmp_query_cpu_model_expansion(CpuModelExpansionType type,
                                                      Error **errp)
 {
     CpuModelExpansionInfo *expansion_info;
-    const QDict *qdict_in = NULL;
     QDict *qdict_out;
     ObjectClass *oc;
     Object *obj;
@@ -188,14 +188,6 @@ CpuModelExpansionInfo *qmp_query_cpu_model_expansion(CpuModelExpansionType type,
         return NULL;
     }
 
-    if (model->props) {
-        qdict_in = qobject_to(QDict, model->props);
-        if (!qdict_in) {
-            error_setg(errp, QERR_INVALID_PARAMETER_TYPE, "props", "dict");
-            return NULL;
-        }
-    }
-
     obj = object_new(object_class_get_name(oc));
 
     riscv_check_if_cpu_available(RISCV_CPU(obj), &local_err);
@@ -205,9 +197,8 @@ CpuModelExpansionInfo *qmp_query_cpu_model_expansion(CpuModelExpansionType type,
         return NULL;
     }
 
-    if (qdict_in) {
-        riscv_cpuobj_validate_qdict_in(obj, model->props, qdict_in,
-                                       &local_err);
+    if (model->props) {
+        riscv_cpuobj_validate_qdict_in(obj, model->props, &local_err);
         if (local_err) {
             error_propagate(errp, local_err);
             object_unref(obj);
diff --git a/target/s390x/cpu_models_sysemu.c b/target/s390x/cpu_models_sysemu.c
index 63981bf36b..ef19724888 100644
--- a/target/s390x/cpu_models_sysemu.c
+++ b/target/s390x/cpu_models_sysemu.c
@@ -101,21 +101,13 @@ static void cpu_model_from_info(S390CPUModel *model, const CpuModelInfo *info,
                                 Error **errp)
 {
     Error *err = NULL;
-    const QDict *qdict = NULL;
+    const QDict *qdict;
     const QDictEntry *e;
     Visitor *visitor;
     ObjectClass *oc;
     S390CPU *cpu;
     Object *obj;
 
-    if (info->props) {
-        qdict = qobject_to(QDict, info->props);
-        if (!qdict) {
-            error_setg(errp, QERR_INVALID_PARAMETER_TYPE, "props", "dict");
-            return;
-        }
-    }
-
     oc = cpu_class_by_name(TYPE_S390_CPU, info->name);
     if (!oc) {
         error_setg(errp, "The CPU definition \'%s\' is unknown.", info->name);
@@ -135,13 +127,14 @@ static void cpu_model_from_info(S390CPUModel *model, const CpuModelInfo *info,
         return;
     }
 
-    if (qdict) {
+    if (info->props) {
         visitor = qobject_input_visitor_new(info->props);
-        if (!visit_start_struct(visitor, NULL, NULL, 0, errp)) {
+        if (!visit_start_struct(visitor, "props", NULL, 0, errp)) {
             visit_free(visitor);
             object_unref(obj);
             return;
         }
+        qdict = qobject_to(QDict, info->props);
         for (e = qdict_first(qdict); e; e = qdict_next(qdict, e)) {
             if (!object_property_set(obj, e->key, visitor, &err)) {
                 break;
diff --git a/tests/qtest/arm-cpu-features.c b/tests/qtest/arm-cpu-features.c
index a8a4c668ad..1daceb2e31 100644
--- a/tests/qtest/arm-cpu-features.c
+++ b/tests/qtest/arm-cpu-features.c
@@ -79,7 +79,7 @@ static const char *resp_get_error(QDict *resp)
     g_assert(_resp);                                                   \
     _error = resp_get_error(_resp);                                    \
     g_assert(_error);                                                  \
-    g_assert(g_str_equal(_error, expected_error));                     \
+    g_assert_cmpstr(_error, ==, expected_error);                       \
     qobject_unref(_resp);                                              \
 })
 
@@ -194,8 +194,8 @@ static void assert_type_full(QTestState *qts)
     g_assert(resp);
     error = resp_get_error(resp);
     g_assert(error);
-    g_assert(g_str_equal(error,
-                         "The requested expansion type is not supported"));
+    g_assert_cmpstr(error, ==,
+                    "The requested expansion type is not supported");
     qobject_unref(resp);
 }
 
@@ -212,8 +212,8 @@ static void assert_bad_props(QTestState *qts, const char *cpu_type)
     g_assert(resp);
     error = resp_get_error(resp);
     g_assert(error);
-    g_assert(g_str_equal(error,
-                         "Invalid parameter type for 'props', expected: dict"));
+    g_assert_cmpstr(error, ==,
+                    "Invalid parameter type for 'props', expected: object");
     qobject_unref(resp);
 }
 
@@ -446,7 +446,7 @@ static void test_query_cpu_model_expansion(const void *data)
     assert_bad_props(qts, "max");
     assert_error(qts, "foo", "The CPU type 'foo' is not a recognized "
                  "ARM CPU type", NULL);
-    assert_error(qts, "max", "Parameter 'not-a-prop' is unexpected",
+    assert_error(qts, "max", "Parameter 'props.not-a-prop' is unexpected",
                  "{ 'not-a-prop': false }");
     assert_error(qts, "host", "The CPU type 'host' requires KVM", NULL);
 
-- 
2.44.0



  reply	other threads:[~2024-03-05 15:00 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-05 14:59 [PATCH 0/4] target: Improve error reporting for CpuModelInfo member @props Markus Armbruster
2024-03-05 14:59 ` Markus Armbruster [this message]
2024-03-06 13:05   ` [PATCH 1/4] target: Simplify type checks " Daniel Henrique Barboza
2024-03-05 14:59 ` [PATCH 2/4] target/i386: Fix query-cpu-model-expansion to reject props Markus Armbruster
2024-03-05 14:59 ` [PATCH 3/4] target: Improve error reporting for CpuModelInfo member @props Markus Armbruster
2024-03-06 13:05   ` Daniel Henrique Barboza
2024-03-05 14:59 ` [PATCH 4/4] target/loongarch: Fix query-cpu-model-expansion to reject props Markus Armbruster

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240305145919.2186971-2-armbru@redhat.com \
    --to=armbru@redhat.com \
    --cc=alistair.francis@wdc.com \
    --cc=bin.meng@windriver.com \
    --cc=david@redhat.com \
    --cc=dbarboza@ventanamicro.com \
    --cc=gaosong@loongson.cn \
    --cc=iii@linux.ibm.com \
    --cc=liwei1518@gmail.com \
    --cc=palmer@dabbelt.com \
    --cc=pbonzini@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-arm@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-riscv@nongnu.org \
    --cc=qemu-s390x@nongnu.org \
    --cc=richard.henderson@linaro.org \
    --cc=thuth@redhat.com \
    --cc=zhiwei_liu@linux.alibaba.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).