On Mon, 1 Sept 2025 at 13:53, Alex Bennée <alex.ben...@linaro.org> wrote:
>
> With the fdt being protected by g_autofree we can skip the goto fail
> and bail out straight away. The only thing we must take care of is
> stealing the pointer in the one case when we do need it to survive.
>
> Signed-off-by: Alex Bennée <alex.ben...@linaro.org>
> ---
> hw/arm/boot.c | 29 ++++++++++++-----------------
> 1 file changed, 12 insertions(+), 17 deletions(-)
>
> diff --git a/hw/arm/boot.c b/hw/arm/boot.c
> index 56fd13b9f7c..749f2d08341 100644
> --- a/hw/arm/boot.c
> +++ b/hw/arm/boot.c
> @@ -519,7 +519,7 @@ int arm_load_dtb(hwaddr addr, const struct arm_boot_info
> *binfo,
> hwaddr addr_limit, AddressSpace *as, MachineState *ms,
> ARMCPU *cpu)
> {
> - void *fdt = NULL;
> + g_autofree void *fdt = NULL;
> int size, rc, n = 0;
> uint32_t acells, scells;
> unsigned int i;
> @@ -673,14 +672,10 @@ int arm_load_dtb(hwaddr addr, const struct
> arm_boot_info *binfo,
>
> if (fdt != ms->fdt) {
> g_free(ms->fdt);
> - ms->fdt = fdt;
> + ms->fdt = g_steal_pointer(&fdt);
> }
>
> return size;
> -> -fail:
> - g_free(fdt);
> - return -1;
> }
Previously, if we get to the end of the function and fdt == ms->fdt
then we continue to use that DTB, and we don't free it.
After this change, if fdt == ms->fdt then we will skip the
g_steal_pointer() and the g_autofree will free the memory,
but leave ms->fdt still pointing to it.
Since arm_load_dtb() is only called once it's a bit unclear
to me whether this can happen -- I think you would need to have
a board-specific arm_boot_info::get_dtb function which returned
the MachineState::fdt pointer. But as this is supposed to
just be a refactoring patch and the previous code clearly was
written to account for the possibility of fdt == ms->fdt,
I think we should continue to handle that case.
thanks
-- PMM
