qemu-riscv
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v9 3/9] machine: Improve is_cpu_type_supported()


From: Philippe Mathieu-Daudé
Subject: Re: [PATCH v9 3/9] machine: Improve is_cpu_type_supported()
Date: Fri, 5 Jan 2024 23:09:06 +0100
User-agent: Mozilla Thunderbird

On 4/12/23 01:47, Gavin Shan wrote:
It's no sense to check the CPU type when mc->valid_cpu_types[0] is
NULL, which is a program error. Raise an assert on this.

A precise hint for the error message is given when mc->valid_cpu_types[0]
is the only valid entry. Besides, enumeration on mc->valid_cpu_types[0]
when we have mutiple valid entries there is avoided to increase the code
readability, as suggested by Philippe Mathieu-Daudé.

Besides, @cc comes from machine->cpu_type or mc->default_cpu_type. For
the later case, it can be NULL and it's also a program error. We should
use assert() in this case.

Signed-off-by: Gavin Shan <gshan@redhat.com>
v9: assert(mc->valid_cpu_types[0] != NULL)                   (Phil)
     assert(cc != NULL);                                      (Phil)
---
  hw/core/machine.c | 20 ++++++++++++++------
  1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/hw/core/machine.c b/hw/core/machine.c
index 1797e002f9..4ae9aaee8e 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -1400,6 +1400,7 @@ static bool is_cpu_type_supported(const MachineState 
*machine, Error **errp)
       * type is provided through '-cpu' option.
       */
      if (mc->valid_cpu_types && machine->cpu_type) {
+        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])) {
                  break;
@@ -1409,20 +1410,27 @@ static bool is_cpu_type_supported(const MachineState 
*machine, Error **errp)
          /* The user specified CPU type isn't valid */
          if (!mc->valid_cpu_types[i]) {
              error_setg(errp, "Invalid CPU type: %s", machine->cpu_type);
-            error_append_hint(errp, "The valid types are: %s",
-                              mc->valid_cpu_types[0]);
-            for (i = 1; mc->valid_cpu_types[i]; i++) {
-                error_append_hint(errp, ", %s", mc->valid_cpu_types[i]);
+            if (!mc->valid_cpu_types[1]) {
+                error_append_hint(errp, "The only valid type is: %s\n",
+                                  mc->valid_cpu_types[0]);
+            } else {
+                error_append_hint(errp, "The valid types are: ");
+                for (i = 0; mc->valid_cpu_types[i]; i++) {
+                    error_append_hint(errp, "%s%s",
+                                      mc->valid_cpu_types[i],
+                                      mc->valid_cpu_types[i + 1] ? ", " : "");
+                }
+                error_append_hint(errp, "\n");
              }
- error_append_hint(errp, "\n");
              return false;
          }
      }
/* Check if CPU type is deprecated and warn if so */
      cc = CPU_CLASS(oc);
-    if (cc && cc->deprecation_note) {
+    assert(cc != NULL);
+    if (cc->deprecation_note) {
          warn_report("CPU model %s is deprecated -- %s",
                      machine->cpu_type, cc->deprecation_note);
      }

Since we were getting:

$ qemu-system-s390x -M none
QEMU 8.2.50 monitor - type 'help' for more information
qemu-system-s390x: ../../hw/core/machine.c:1444: _Bool is_cpu_type_supported(const MachineState *, Error **): Assertion `cc != NULL' failed.
Aborted (core dumped)

I added a check on mc->valid_cpu_types before calling
is_cpu_type_supported() in the previous patch. See commit acbadc5a29.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]