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

[bibo mao]

On 2025/3/14 下午5:38, Markus Armbruster wrote:

bibo mao <maob...@loongson.cn> writes:

On 2025/3/14 下午5:11, 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>

[...]

@@ -1001,9 +999,8 @@ static void virt_cpu_plug(HotplugHandler *hotplug_dev,

         cpu_slot = virt_find_cpu_slot(MACHINE(lvms), cpu->phy_id);
         cpu_slot->cpu = CPU(dev);
         if (lvms->ipi) {
             hotplug_handler_plug(HOTPLUG_HANDLER(lvms->ipi), dev, &err);
             if (err) {
                 error_propagate(errp, err);
                 return;
             }
         }

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

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

return;

Better make this work exactly like the other checks above, and drop the
final return, which serves no purpose:

             if (err) {
                 error_propagate(errp, err);
                 return;
             }
         }

Here is report from commandline, it say err NULL check is not necessary
spatch --sp-file scripts/coccinelle/error_propagate_null.cocci --dir
hw/loongarch/

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

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

This change is correct.  The change I suggests is also correct.  I
prefer mine because it's less easy to screw up.  If you add another
check at the end, my version keeps working, while yours needs an update,
which is easy to miss.

I see until checking file scripts/coccinelle/error_propagate_null.cocci
-if (L) {
     error_propagate(E, L);
-}

you are error log expert, of source do with your suggestion -:)
And thanks for your earnest explanation.

Regards
Bibo Mao