Re: [PATCH 01/23] hw/ppc/e500: Do not leak struct boot_info

[Bernhard Beschow <shentey@gmail.com>]

Am 23. September 2024 10:02:10 UTC schrieb BALATON Zoltan <balaton@eik.bme.hu>:
>On Mon, 23 Sep 2024, Bernhard Beschow wrote:
>> The struct is allocated once with g_new0() but never free()'d. Fix the leakage
>> by adding an attribute to struct PPCE500MachineState which avoids the
>> allocation.
>> 
>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>> ---
>> hw/ppc/e500.h |  8 ++++++++
>> hw/ppc/e500.c | 17 ++++-------------
>> 2 files changed, 12 insertions(+), 13 deletions(-)
>> 
>> diff --git a/hw/ppc/e500.h b/hw/ppc/e500.h
>> index 8c09ef92e4..557ce6ad93 100644
>> --- a/hw/ppc/e500.h
>> +++ b/hw/ppc/e500.h
>> @@ -5,10 +5,18 @@
>> #include "hw/platform-bus.h"
>> #include "qom/object.h"
>> 
>> +typedef struct boot_info {
>> +    uint32_t dt_base;
>> +    uint32_t dt_size;
>> +    uint32_t entry;
>> +} boot_info;
>> +
>> struct PPCE500MachineState {
>>     /*< private >*/
>
>While at it you could remove these private markers...

Will do.

>
>>     MachineState parent_obj;
>> 
>> +    boot_info boot_info;
>> +
>
>...and drop the new line here so only the parent_obj is followed by a new line as was suggested as reccomended style.

I'll merge struct boot_info below as Cédric suggested.

Best regards,
Bernhard

>
>Regatds,
>BALATON Zoltan
>
>>     /* points to instance of TYPE_PLATFORM_BUS_DEVICE if
>>      * board supports dynamic sysbus devices
>>      */
>> diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
>> index 3bd12b54ab..75b051009f 100644
>> --- a/hw/ppc/e500.c
>> +++ b/hw/ppc/e500.c
>> @@ -80,13 +80,6 @@
>> 
>> #define PLATFORM_CLK_FREQ_HZ       (400 * 1000 * 1000)
>> 
>> -struct boot_info
>> -{
>> -    uint32_t dt_base;
>> -    uint32_t dt_size;
>> -    uint32_t entry;
>> -};
>> -
>> static uint32_t *pci_map_create(void *fdt, uint32_t mpic, int first_slot,
>>                                 int nr_slots, int *len)
>> {
>> @@ -919,7 +912,6 @@ void ppce500_init(MachineState *machine)
>>     bool kernel_as_payload;
>>     hwaddr bios_entry = 0;
>>     target_long payload_size;
>> -    struct boot_info *boot_info = NULL;
>>     int dt_size;
>>     int i;
>>     unsigned int smp_cpus = machine->smp.cpus;
>> @@ -974,9 +966,8 @@ void ppce500_init(MachineState *machine)
>>         /* Register reset handler */
>>         if (!i) {
>>             /* Primary CPU */
>> -            boot_info = g_new0(struct boot_info, 1);
>>             qemu_register_reset(ppce500_cpu_reset, cpu);
>> -            env->load_info = boot_info;
>> +            env->load_info = &pms->boot_info;
>>         } else {
>>             /* Secondary CPUs */
>>             qemu_register_reset(ppce500_cpu_reset_sec, cpu);
>> @@ -1274,9 +1265,9 @@ void ppce500_init(MachineState *machine)
>>     }
>>     assert(dt_size < DTB_MAX_SIZE);
>> 
>> -    boot_info->entry = bios_entry;
>> -    boot_info->dt_base = dt_base;
>> -    boot_info->dt_size = dt_size;
>> +    pms->boot_info.entry = bios_entry;
>> +    pms->boot_info.dt_base = dt_base;
>> +    pms->boot_info.dt_size = dt_size;
>> }
>> 
>> static void e500_ccsr_initfn(Object *obj)
>>