Re: [PATCH 2/2] hw/riscv: Don't add empty bootargs to device tree

[Alistair Francis]

On Thu, Apr 21, 2022 at 3:58 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> From: Bin Meng <bin.meng@windriver.com>
>
> Commit 7c28f4da20e5 ("RISC-V: Don't add NULL bootargs to device-tree")
> tried to avoid adding *NULL* bootargs to device tree, but unfortunately
> the changes were entirely useless, due to MachineState::kernel_cmdline
> can't be NULL at all as the default value is given as an empty string.
> (see hw/core/machine.c::machine_initfn()).
>
> Note the wording of *NULL* bootargs is wrong. It can't be NULL otherwise
> a segfault had already been observed by dereferencing the NULL pointer.
> It should be worded as *empty" bootargs.
>
> Fixes: 7c28f4da20e5 ("RISC-V: Don't add NULL bootargs to device-tree")
> Signed-off-by: Bin Meng <bin.meng@windriver.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>
>  hw/riscv/microchip_pfsoc.c | 2 +-
>  hw/riscv/sifive_u.c        | 2 +-
>  hw/riscv/spike.c           | 2 +-
>  hw/riscv/virt.c            | 2 +-
>  4 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/hw/riscv/microchip_pfsoc.c b/hw/riscv/microchip_pfsoc.c
> index cafd1fc9ae..10a5d0e501 100644
> --- a/hw/riscv/microchip_pfsoc.c
> +++ b/hw/riscv/microchip_pfsoc.c
> @@ -571,7 +571,7 @@ static void 
> microchip_icicle_kit_machine_init(MachineState *machine)
>                                    "linux,initrd-end", end);
>          }
>
> -        if (machine->kernel_cmdline) {
> +        if (machine->kernel_cmdline && *machine->kernel_cmdline) {
>              qemu_fdt_setprop_string(machine->fdt, "/chosen",
>                                      "bootargs", machine->kernel_cmdline);
>          }
> diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
> index 7fbc7dea42..cc8c7637cb 100644
> --- a/hw/riscv/sifive_u.c
> +++ b/hw/riscv/sifive_u.c
> @@ -511,7 +511,7 @@ static void create_fdt(SiFiveUState *s, const MemMapEntry 
> *memmap,
>      g_free(nodename);
>
>  update_bootargs:
> -    if (cmdline) {
> +    if (cmdline && *cmdline) {
>          qemu_fdt_setprop_string(fdt, "/chosen", "bootargs", cmdline);
>      }
>  }
> diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
> index 1562b000bb..068ba3493e 100644
> --- a/hw/riscv/spike.c
> +++ b/hw/riscv/spike.c
> @@ -177,7 +177,7 @@ static void create_fdt(SpikeState *s, const MemMapEntry 
> *memmap,
>      qemu_fdt_add_subnode(fdt, "/chosen");
>      qemu_fdt_setprop_string(fdt, "/chosen", "stdout-path", "/htif");
>
> -    if (cmdline) {
> +    if (cmdline && *cmdline) {
>          qemu_fdt_setprop_string(fdt, "/chosen", "bootargs", cmdline);
>      }
>  }
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index da50cbed43..a628a3abdf 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -998,7 +998,7 @@ static void create_fdt(RISCVVirtState *s, const 
> MemMapEntry *memmap,
>      create_fdt_flash(s, memmap);
>
>  update_bootargs:
> -    if (cmdline) {
> +    if (cmdline && *cmdline) {
>          qemu_fdt_setprop_string(mc->fdt, "/chosen", "bootargs", cmdline);
>      }
>  }
> --
> 2.25.1
>
>