Re: [PATCH v1 1/1] hw/riscv/boot: Check the error of fdt_pack()

[Peter Maydell]

On Wed, 14 Jul 2021 at 08:22, Alistair Francis <alistair.francis@wdc.com> wrote:
>
> Coverity reports that we don't check the error result of fdt_pack(), so
> let's save the result and assert that it is 0.
>
> Fixes: Coverity CID 1458136
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>  hw/riscv/boot.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c
> index 0d38bb7426..25406a3f67 100644
> --- a/hw/riscv/boot.c
> +++ b/hw/riscv/boot.c
> @@ -182,7 +182,7 @@ uint32_t riscv_load_fdt(hwaddr dram_base, uint64_t 
> mem_size, void *fdt)
>  {
>      uint32_t temp, fdt_addr;
>      hwaddr dram_end = dram_base + mem_size;
> -    int fdtsize = fdt_totalsize(fdt);
> +    int ret, fdtsize = fdt_totalsize(fdt);
>
>      if (fdtsize <= 0) {
>          error_report("invalid device-tree");
> @@ -198,7 +198,8 @@ uint32_t riscv_load_fdt(hwaddr dram_base, uint64_t 
> mem_size, void *fdt)
>      temp = MIN(dram_end, 3072 * MiB);
>      fdt_addr = QEMU_ALIGN_DOWN(temp - fdtsize, 16 * MiB);
>
> -    fdt_pack(fdt);
> +    ret = fdt_pack(fdt);
> +    g_assert(ret == 0);
>      /* copy in the device tree */
>      qemu_fdt_dumpdtb(fdt, fdtsize);

Are we in the same situation as spapr.c where we believe the call
will only fail if the fdt was corrupt somehow? If so, it would be
nice to also have the comment from spapr.c:
        /* Should only fail if we've built a corrupted tree */

thanks
-- PMM