Daniel Henrique Barboza <danielhb...@gmail.com> writes: > On 3/28/23 06:53, Markus Armbruster wrote: >> Daniel Henrique Barboza <danielhb...@gmail.com> writes:
[...] >>> I believe we can improve the ARM boot code to not create ms->fdt at init(), >>> leaving it unassigned, and make get_dtb() return the machine FDT on a common >>> "void *" pointer. That would spare us from having go g_free(ms->fdt) to >>> avoid >>> leaks and we would assign ms->fdt at the end of arm_load_dtb() normally. I >>> made >>> a quick attempt at that but the ARM init() code is a little tricker than >>> I've >>> anticipated. I might have a crack at it later. >> >> Do we want a quick interim fix for 8.0? >> Have a careful look at the untested patch below. >> >>> Thanks, >>> >>> Daniel >>> >>> >>>> >>>> } >>>> >>>>> @@ -689,7 +696,8 @@ int arm_load_dtb(hwaddr addr, const struct >>>>> arm_boot_info *binfo, >>>>> qemu_register_reset_nosnapshotload(qemu_fdt_randomize_seeds, >>>>> rom_ptr_for_as(as, addr, size)); >>>>> - g_free(fdt); >>>>> + /* Set ms->fdt for 'dumpdtb' QMP/HMP command */ >>>>> + ms->fdt = fdt; >>>>> return size; >>>> >> diff --git a/hw/arm/boot.c b/hw/arm/boot.c >> index 50e5141116..54f6a3e0b3 100644 >> --- a/hw/arm/boot.c >> +++ b/hw/arm/boot.c >> @@ -689,7 +689,10 @@ int arm_load_dtb(hwaddr addr, const struct >> arm_boot_info *binfo, >> qemu_register_reset_nosnapshotload(qemu_fdt_randomize_seeds, >> rom_ptr_for_as(as, addr, size)); >> - g_free(fdt); >> + if (fdt != ms->fdt) { >> + g_free(ms->fdt); >> + ms->fdt = fdt; >> + } >> return size; > > This looks better than what I've been proposing here because it centers > everything in > the same spot. It'll also make it easier to change/remove it when we have the > chance > to take a look at the ARM boot code. > > Just tested it here and it works fine. Feel free to format it into a patch > and send > it. I'll give my r-b. I'm going to send it as v3 with your S-o-B, because I'm stealing most of your commit message.