diff mbox series

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

Message ID 20250317022922.802988-3-maobibo@loongson.cn (mailing list archive)
State New
Headers show
Series target/loongarch: Solve some issues reported from coccinelle | expand

Commit Message

Bibo Mao March 17, 2025, 2:29 a.m. UTC 
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 <maobibo@loongson.cn>
---
 hw/loongarch/virt.c | 28 +++++++++++-----------------
 1 file changed, 11 insertions(+), 17 deletions(-)

Comments

Markus Armbruster March 17, 2025, 6:08 a.m. UTC | #1 
Bibo Mao <maobibo@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 <maobibo@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.

> -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);
 }

>  }
>  
>  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;
>          }
>      }
Bibo Mao March 17, 2025, 6:48 a.m. UTC | #2 
On 2025/3/17 下午2:08, Markus Armbruster wrote:
> Bibo Mao <maobibo@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 <maobibo@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;
>>           }
>>       }
>
diff mbox series

Patch

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);
-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);
 }
 
 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;
         }
     }