virt: Remove unnecessary NULL pointer checking

bibo mao Thu, 13 Mar 2025 04:25:21 -0700



On 2025/3/13 下午6:32, 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.

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

diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c
index a5840ff968..ab951fc642 100644
--- a/hw/loongarch/virt.c
+++ b/hw/loongarch/virt.c
@@ -913,9 +913,7 @@ static void virt_cpu_pre_plug(HotplugHandler *hotplug_dev,
      cs->cpu_index = cpu_slot - ms->possible_cpus->cpus;
      numa_cpu_pre_plug(cpu_slot, dev, &err);
  out:
-    if (err) {
-        error_propagate(errp, err);
-    }
+    error_propagate(errp, err);
  }

static void virt_cpu_unplug_request(HotplugHandler *hotplug_dev,

@@ -935,9 +933,7 @@ static void virt_cpu_unplug_request(HotplugHandler 
*hotplug_dev,
      }

hotplug_handler_unplug_request(HOTPLUG_HANDLER(lvms->acpi_ged), dev, &err);

-    if (err) {
-        error_propagate(errp, err);
-    }
+    error_propagate(errp, err);
  }


This looks correct.  But I believe we can eliminate error propagation
here.  Untested patch appended.

Sure, will eliminate error propagation and return earlier.

Previously I always consider correctness and neglect these coding styledetails, that is actually another gap -:)


Regards
Bibo Mao

static void virt_cpu_unplug(HotplugHandler *hotplug_dev,

@@ -1001,9 +997,7 @@ static void virt_cpu_plug(HotplugHandler *hotplug_dev,

if (lvms->acpi_ged) {

          hotplug_handler_plug(HOTPLUG_HANDLER(lvms->acpi_ged), dev, &err);
-        if (err) {
-            error_propagate(errp, err);
-        }
+        error_propagate(errp, err);
      }

return;


Here, I'd recommend

        if (lvms->acpi_ged) {
            hotplug_handler_plug(HOTPLUG_HANDLER(lvms->acpi_ged), dev, &err);
            if (err) {
                error_propagate(errp, err);
   +            return;
            }

because it makes future screwups harder.


diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c
index a5840ff968..4674bd9163 100644
--- a/hw/loongarch/virt.c
+++ b/hw/loongarch/virt.c
@@ -859,30 +859,29 @@ static void virt_cpu_pre_plug(HotplugHandler *hotplug_dev,
      LoongArchCPU *cpu = LOONGARCH_CPU(dev);
      CPUState *cs = CPU(dev);
      CPUArchId *cpu_slot;
-    Error *err = NULL;
      LoongArchCPUTopo topo;
      int arch_id;

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 +890,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 */
@@ -911,33 +910,24 @@ static void virt_cpu_pre_plug(HotplugHandler *hotplug_dev,
      cpu->env.address_space_iocsr = &lvms->as_iocsr;
      cpu->phy_id = cpu_slot->arch_id;
      cs->cpu_index = cpu_slot - ms->possible_cpus->cpus;
-    numa_cpu_pre_plug(cpu_slot, dev, &err);
-out:
-    if (err) {
-        error_propagate(errp, err);
-    }
+    numa_cpu_pre_plug(cpu_slot, dev, errp);
  }

static void virt_cpu_unplug_request(HotplugHandler *hotplug_dev,

                                      DeviceState *dev, Error **errp)
  {
      LoongArchVirtMachineState *lvms = LOONGARCH_VIRT_MACHINE(hotplug_dev);
-    Error *err = NULL;
      LoongArchCPU *cpu = LOONGARCH_CPU(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);
-    }
+    hotplug_handler_unplug_request(HOTPLUG_HANDLER(lvms->acpi_ged), dev, errp);
  }

static void virt_cpu_unplug(HotplugHandler *hotplug_dev,

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

Reply via email to