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,
-- 
2.35.1

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:

        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);

thanks
-- PMM

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