Re: [PATCH] hw/ppc/ppc405_boards: Initialize g_autofree pointer

[Bernhard Beschow]

Am 5. April 2022 12:00:19 UTC schrieb Peter Maydell <peter.maydell@linaro.org>:
>On Tue, 5 Apr 2022 at 12:32, Bernhard Beschow <shentey@gmail.com> wrote:
>>
>> Resolves the only compiler warning when building a full QEMU under Arch 
>> Linux:
>>
>>   Compiling C object libqemu-ppc-softmmu.fa.p/hw_ppc_ppc405_boards.c.o
>>   In file included from /usr/include/glib-2.0/glib.h:114,
>>                    from qemu/include/glib-compat.h:32,
>>                    from qemu/include/qemu/osdep.h:132,
>>                    from ../src/hw/ppc/ppc405_boards.c:25:
>>   ../src/hw/ppc/ppc405_boards.c: In function ‘ref405ep_init’:
>>   /usr/include/glib-2.0/glib/glib-autocleanups.h:28:3: warning: ‘filename’ 
>> may be used uninitialized in this function [-Wmaybe-uninitialized]
>>      28 |   g_free (*pp);
>>         |   ^~~~~~~~~~~~
>>   ../src/hw/ppc/ppc405_boards.c:265:26: note: ‘filename’ was declared here
>>     265 |         g_autofree char *filename;
>>         |                          ^~~~~~~~
>>
>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>> ---
>>  hw/ppc/ppc405_boards.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/hw/ppc/ppc405_boards.c b/hw/ppc/ppc405_boards.c
>> index 7e1a4ac955..326353ea25 100644
>> --- a/hw/ppc/ppc405_boards.c
>> +++ b/hw/ppc/ppc405_boards.c
>> @@ -262,7 +262,7 @@ static void ref405ep_init(MachineState *machine)
>>      /* allocate and load BIOS */
>>      if (machine->firmware) {
>>          MemoryRegion *bios = g_new(MemoryRegion, 1);
>> -        g_autofree char *filename;
>> +        g_autofree char *filename = NULL;
>>          long bios_size;
>>
>>          memory_region_init_rom(bios, NULL, "ef405ep.bios", BIOS_SIZE,
>
>The compiler's wrong here, because there's no way to get to the free
>without passing through the actual initialization:

Yep. It breaks compilation with -Werror, though, which is useful for 
development.

>
>        filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, machine->firmware);
>
>I think I would prefer a fix which hoisted that up to the declaration,
>rather than setting it to NULL and then unconditionally overwriting that
>(which some future compiler version might notice and warn about):
>
> g_autofree char *filename = qemu_find_file(QEMU_FILE_TYPE_BIOS,
>machine->firmware);

Ack - I prefer that solution and I'll submit v2.

I'm often confused as to when to use RAII in QEMU and when not to.

Best regards,
Bernhard

>
>thanks
>-- PMM