1. Patchew
  2. QEMU
  3. [PATCH v3 0/8] Support VBOOTROM to ast2700fc machine
  4. View patch

[PATCH v3 5/8] hw/arm/aspeed_ast27x0-fc: Drop dead return checks

Jamin Lin via posted 8 patches 3 days, 10 hours ago
Maintainers: "Cédric Le Goater" <clg@kaod.org>, Peter Maydell <peter.maydell@linaro.org>, Steven Lee <steven_lee@aspeedtech.com>, Troy Lee <leetroy@gmail.com>, Jamin Lin <jamin_lin@aspeedtech.com>, Andrew Jeffery <andrew@codeconstruct.com.au>, Joel Stanley <joel@jms.id.au>
[PATCH v3 5/8] hw/arm/aspeed_ast27x0-fc: Drop dead return checks
Posted by Jamin Lin via 3 days, 10 hours ago 
1. object_property_set_link() can return false only when it fails, and it
sets an error when it fails. Since passing &error_abort causes an abort,
the function never returns false, and the return statement is effectively
dead code.
2. object_property_set_int() is considered as a routine which shouldn't fail.
So the common practice in models is to pass &error_abort and ignore the returned value.
https://patchwork.kernel.org/project/qemu-devel/patch/20250717034054.1903991-3-jamin_lin@aspeedtech.com/#26540626

No functional change.

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
---
 hw/arm/aspeed_ast27x0-fc.c | 43 +++++++++++++-------------------------
 1 file changed, 14 insertions(+), 29 deletions(-)

diff --git a/hw/arm/aspeed_ast27x0-fc.c b/hw/arm/aspeed_ast27x0-fc.c
index 7087be4288..ebf3784df5 100644
--- a/hw/arm/aspeed_ast27x0-fc.c
+++ b/hw/arm/aspeed_ast27x0-fc.c
@@ -74,19 +74,12 @@ static void ast2700fc_ca35_init(MachineState *machine)
                                 AST2700FC_BMC_RAM_SIZE, &error_abort)) {
         return;
     }
-    if (!object_property_set_link(OBJECT(&s->ca35), "memory",
-                                  OBJECT(&s->ca35_memory),
-                                  &error_abort)) {
-        return;
-    };
-    if (!object_property_set_link(OBJECT(&s->ca35), "dram",
-                                  OBJECT(&s->ca35_dram), &error_abort)) {
-        return;
-    }
-    if (!object_property_set_int(OBJECT(&s->ca35), "ram-size",
-                                 AST2700FC_BMC_RAM_SIZE, &error_abort)) {
-        return;
-    }
+    object_property_set_link(OBJECT(&s->ca35), "memory",
+                             OBJECT(&s->ca35_memory), &error_abort);
+    object_property_set_link(OBJECT(&s->ca35), "dram", OBJECT(&s->ca35_dram),
+                             &error_abort);
+    object_property_set_int(OBJECT(&s->ca35), "ram-size",
+                            AST2700FC_BMC_RAM_SIZE, &error_abort);
 
     for (int i = 0; i < sc->macs_num; i++) {
         if (!qemu_configure_nic_device(DEVICE(&soc->ftgmac100[i]),
@@ -94,14 +87,10 @@ static void ast2700fc_ca35_init(MachineState *machine)
             break;
         }
     }
-    if (!object_property_set_int(OBJECT(&s->ca35), "hw-strap1",
-                                 AST2700FC_HW_STRAP1, &error_abort)) {
-        return;
-    }
-    if (!object_property_set_int(OBJECT(&s->ca35), "hw-strap2",
-                                 AST2700FC_HW_STRAP2, &error_abort)) {
-        return;
-    }
+    object_property_set_int(OBJECT(&s->ca35), "hw-strap1",
+                            AST2700FC_HW_STRAP1, &error_abort);
+    object_property_set_int(OBJECT(&s->ca35), "hw-strap2",
+                            AST2700FC_HW_STRAP2, &error_abort);
     aspeed_soc_uart_set_chr(soc, ASPEED_DEV_UART12, serial_hd(0));
     if (!qdev_realize(DEVICE(&s->ca35), NULL, &error_abort)) {
         return;
@@ -133,10 +122,8 @@ static void ast2700fc_ssp_init(MachineState *machine)
                        UINT64_MAX);
 
     qdev_connect_clock_in(DEVICE(&s->ssp), "sysclk", s->ssp_sysclk);
-    if (!object_property_set_link(OBJECT(&s->ssp), "memory",
-                                  OBJECT(&s->ssp_memory), &error_abort)) {
-        return;
-    }
+    object_property_set_link(OBJECT(&s->ssp), "memory",
+                             OBJECT(&s->ssp_memory), &error_abort);
 
     soc = ASPEED_SOC(&s->ssp);
     aspeed_soc_uart_set_chr(soc, ASPEED_DEV_UART4, serial_hd(1));
@@ -157,10 +144,8 @@ static void ast2700fc_tsp_init(MachineState *machine)
                        UINT64_MAX);
 
     qdev_connect_clock_in(DEVICE(&s->tsp), "sysclk", s->tsp_sysclk);
-    if (!object_property_set_link(OBJECT(&s->tsp), "memory",
-                                  OBJECT(&s->tsp_memory), &error_abort)) {
-        return;
-    }
+    object_property_set_link(OBJECT(&s->tsp), "memory",
+                             OBJECT(&s->tsp_memory), &error_abort);
 
     soc = ASPEED_SOC(&s->tsp);
     aspeed_soc_uart_set_chr(soc, ASPEED_DEV_UART7, serial_hd(2));
-- 
2.43.0
Re: [PATCH v3 5/8] hw/arm/aspeed_ast27x0-fc: Drop dead return checks
Posted by Cédric Le Goater 2 days, 23 hours ago 
On 9/25/25 07:05, Jamin Lin wrote:
> 1. object_property_set_link() can return false only when it fails, and it
> sets an error when it fails. Since passing &error_abort causes an abort,
> the function never returns false, and the return statement is effectively
> dead code.
> 2. object_property_set_int() is considered as a routine which shouldn't fail.
> So the common practice in models is to pass &error_abort and ignore the returned value.
> https://patchwork.kernel.org/project/qemu-devel/patch/20250717034054.1903991-3-jamin_lin@aspeedtech.com/#26540626
> 
> No functional change.
> 
> Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>


Reviewed-by: Cédric Le Goater <clg@redhat.com>

Thanks,

C.


> ---
>   hw/arm/aspeed_ast27x0-fc.c | 43 +++++++++++++-------------------------
>   1 file changed, 14 insertions(+), 29 deletions(-)
> 
> diff --git a/hw/arm/aspeed_ast27x0-fc.c b/hw/arm/aspeed_ast27x0-fc.c
> index 7087be4288..ebf3784df5 100644
> --- a/hw/arm/aspeed_ast27x0-fc.c
> +++ b/hw/arm/aspeed_ast27x0-fc.c
> @@ -74,19 +74,12 @@ static void ast2700fc_ca35_init(MachineState *machine)
>                                   AST2700FC_BMC_RAM_SIZE, &error_abort)) {
>           return;
>       }
> -    if (!object_property_set_link(OBJECT(&s->ca35), "memory",
> -                                  OBJECT(&s->ca35_memory),
> -                                  &error_abort)) {
> -        return;
> -    };
> -    if (!object_property_set_link(OBJECT(&s->ca35), "dram",
> -                                  OBJECT(&s->ca35_dram), &error_abort)) {
> -        return;
> -    }
> -    if (!object_property_set_int(OBJECT(&s->ca35), "ram-size",
> -                                 AST2700FC_BMC_RAM_SIZE, &error_abort)) {
> -        return;
> -    }
> +    object_property_set_link(OBJECT(&s->ca35), "memory",
> +                             OBJECT(&s->ca35_memory), &error_abort);
> +    object_property_set_link(OBJECT(&s->ca35), "dram", OBJECT(&s->ca35_dram),
> +                             &error_abort);
> +    object_property_set_int(OBJECT(&s->ca35), "ram-size",
> +                            AST2700FC_BMC_RAM_SIZE, &error_abort);
>   
>       for (int i = 0; i < sc->macs_num; i++) {
>           if (!qemu_configure_nic_device(DEVICE(&soc->ftgmac100[i]),
> @@ -94,14 +87,10 @@ static void ast2700fc_ca35_init(MachineState *machine)
>               break;
>           }
>       }
> -    if (!object_property_set_int(OBJECT(&s->ca35), "hw-strap1",
> -                                 AST2700FC_HW_STRAP1, &error_abort)) {
> -        return;
> -    }
> -    if (!object_property_set_int(OBJECT(&s->ca35), "hw-strap2",
> -                                 AST2700FC_HW_STRAP2, &error_abort)) {
> -        return;
> -    }
> +    object_property_set_int(OBJECT(&s->ca35), "hw-strap1",
> +                            AST2700FC_HW_STRAP1, &error_abort);
> +    object_property_set_int(OBJECT(&s->ca35), "hw-strap2",
> +                            AST2700FC_HW_STRAP2, &error_abort);
>       aspeed_soc_uart_set_chr(soc, ASPEED_DEV_UART12, serial_hd(0));
>       if (!qdev_realize(DEVICE(&s->ca35), NULL, &error_abort)) {
>           return;
> @@ -133,10 +122,8 @@ static void ast2700fc_ssp_init(MachineState *machine)
>                          UINT64_MAX);
>   
>       qdev_connect_clock_in(DEVICE(&s->ssp), "sysclk", s->ssp_sysclk);
> -    if (!object_property_set_link(OBJECT(&s->ssp), "memory",
> -                                  OBJECT(&s->ssp_memory), &error_abort)) {
> -        return;
> -    }
> +    object_property_set_link(OBJECT(&s->ssp), "memory",
> +                             OBJECT(&s->ssp_memory), &error_abort);
>   
>       soc = ASPEED_SOC(&s->ssp);
>       aspeed_soc_uart_set_chr(soc, ASPEED_DEV_UART4, serial_hd(1));
> @@ -157,10 +144,8 @@ static void ast2700fc_tsp_init(MachineState *machine)
>                          UINT64_MAX);
>   
>       qdev_connect_clock_in(DEVICE(&s->tsp), "sysclk", s->tsp_sysclk);
> -    if (!object_property_set_link(OBJECT(&s->tsp), "memory",
> -                                  OBJECT(&s->tsp_memory), &error_abort)) {
> -        return;
> -    }
> +    object_property_set_link(OBJECT(&s->tsp), "memory",
> +                             OBJECT(&s->tsp_memory), &error_abort);
>   
>       soc = ASPEED_SOC(&s->tsp);
>       aspeed_soc_uart_set_chr(soc, ASPEED_DEV_UART7, serial_hd(2));

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.