Re: [PATCH v3 2/3] hw/loongarch/virt: Remove unnecessary NULL pointer

[bibo mao]

On 2025/3/17 下午2:08, Markus Armbruster wrote:
Bibo Mao <maob...@loongson.cn> writes:

There is NULL pointer checking function error_propagate() already,
it is not necessary to add checking for function parameter. Here remove
NULL pointer checking with function parameter.

Since function will return directly when there is error report, this
patch removes combination about error_setg() and error_propagate(),
error_setg() with dest error object is used directly such as:

   error_setg(err);                 -------->      error_setg(errp);
   error_propagate(errp, err);                     return;
   return;

Signed-off-by: Bibo Mao <maob...@loongson.cn>
---
  hw/loongarch/virt.c | 28 +++++++++++-----------------
  1 file changed, 11 insertions(+), 17 deletions(-)

diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c
index a5840ff968..1fd91f94b5 100644
--- a/hw/loongarch/virt.c
+++ b/hw/loongarch/virt.c
@@ -865,24 +865,24 @@ static void virt_cpu_pre_plug(HotplugHandler *hotplug_dev,
  
      if (lvms->acpi_ged) {
          if ((cpu->thread_id < 0) || (cpu->thread_id >= ms->smp.threads)) {
-            error_setg(&err,
+            error_setg(errp,
                         "Invalid thread-id %u specified, must be in range 
1:%u",
                         cpu->thread_id, ms->smp.threads - 1);
-            goto out;
+            return;
          }
  
          if ((cpu->core_id < 0) || (cpu->core_id >= ms->smp.cores)) {
-            error_setg(&err,
+            error_setg(errp,
                         "Invalid core-id %u specified, must be in range 1:%u",
                         cpu->core_id, ms->smp.cores - 1);
-            goto out;
+            return;
          }
  
          if ((cpu->socket_id < 0) || (cpu->socket_id >= ms->smp.sockets)) {
-            error_setg(&err,
+            error_setg(errp,
                         "Invalid socket-id %u specified, must be in range 
1:%u",
                         cpu->socket_id, ms->smp.sockets - 1);
-            goto out;
+            return;
          }
  
          topo.socket_id = cpu->socket_id;
@@ -891,11 +891,11 @@ static void virt_cpu_pre_plug(HotplugHandler *hotplug_dev,
          arch_id =  virt_get_arch_id_from_topo(ms, &topo);
          cpu_slot = virt_find_cpu_slot(ms, arch_id);
          if (CPU(cpu_slot->cpu)) {
-            error_setg(&err,
+            error_setg(errp,
                         "cpu(id%d=%d:%d:%d) with arch-id %" PRIu64 " exists",
                         cs->cpu_index, cpu->socket_id, cpu->core_id,
                         cpu->thread_id, cpu_slot->arch_id);
-            goto out;
+            return;
          }
      } else {
          /* For cold-add cpu, find empty cpu slot */
@@ -912,10 +912,6 @@ static void virt_cpu_pre_plug(HotplugHandler *hotplug_dev,
      cpu->phy_id = cpu_slot->arch_id;
      cs->cpu_index = cpu_slot - ms->possible_cpus->cpus;
      numa_cpu_pre_plug(cpu_slot, dev, &err);

You need to pass errp instead of &err now.
yes, should pass errp and remove local variable Error err.


-out:
-    if (err) {
-        error_propagate(errp, err);
-    }
  }
  
  static void virt_cpu_unplug_request(HotplugHandler *hotplug_dev,
@@ -927,17 +923,14 @@ static void virt_cpu_unplug_request(HotplugHandler 
*hotplug_dev,
      CPUState *cs = CPU(dev);
  
      if (cs->cpu_index == 0) {
-        error_setg(&err, "hot-unplug of boot cpu(id%d=%d:%d:%d) not supported",
+        error_setg(errp, "hot-unplug of boot cpu(id%d=%d:%d:%d) not supported",
                     cs->cpu_index, cpu->socket_id,
                     cpu->core_id, cpu->thread_id);
-        error_propagate(errp, err);
          return;
      }
  
      hotplug_handler_unplug_request(HOTPLUG_HANDLER(lvms->acpi_ged), dev, &err);
-    if (err) {
-        error_propagate(errp, err);
-    }
+    error_propagate(errp, err);

Missed this cleanup:

        hotplug_handler_unplug_request(HOTPLUG_HANDLER(lvms->acpi_ged), dev, 
errp);
  }
will do. With that, almost all local variable Error *err is removed. And 
that is cleaner.

Regards
Bibo Mao

  }
  
  static void virt_cpu_unplug(HotplugHandler *hotplug_dev,
@@ -1003,6 +996,7 @@ static void virt_cpu_plug(HotplugHandler *hotplug_dev,
          hotplug_handler_plug(HOTPLUG_HANDLER(lvms->acpi_ged), dev, &err);
          if (err) {
              error_propagate(errp, err);
+            return;
          }
      }