Re: [PATCH] tests/unit/test-io-channel-command: Silence GCC error "maybe-uninitialized"

[Laurent Vivier <laurent@vivier.eu>]

Le 02/11/2022 à 21:24, Alex Bennée a écrit :
> 
> Bernhard Beschow <shentey@gmail.com> writes:
> 
>> GCC issues a false positive warning, resulting in build failure with -Werror:
>>
>>    In file included from /usr/lib/glib-2.0/include/glibconfig.h:9,
>>                     from /usr/include/glib-2.0/glib/gtypes.h:34,
>>                     from /usr/include/glib-2.0/glib/galloca.h:34,
>>                     from /usr/include/glib-2.0/glib.h:32,
>>                     from ../src/include/glib-compat.h:32,
>>                     from ../src/include/qemu/osdep.h:144,
>>                     from ../src/tests/unit/test-io-channel-command.c:21:
>>    /usr/include/glib-2.0/glib/gmacros.h: In function ‘test_io_channel_command_fifo’:
>>    /usr/include/glib-2.0/glib/gmacros.h:1333:105: error: ‘dstargv’ may be used uninitialized [-Werror=maybe-uninitialized]
>>     1333 |   static G_GNUC_UNUSED inline void _GLIB_AUTO_FUNC_NAME(TypeName) (TypeName *_ptr) { if (*_ptr != none) (func) (*_ptr); }     \
>>          |                                                                                                         ^
>>    ../src/tests/unit/test-io-channel-command.c:39:19: note: ‘dstargv’ was declared here
>>       39 |     g_auto(GStrv) dstargv;
>>          |                   ^~~~~~~
>>    /usr/include/glib-2.0/glib/gmacros.h:1333:105: error: ‘srcargv’ may
>> be used uninitialized [-Werror=maybe-uninitialized]
>>     1333 | static G_GNUC_UNUSED inline void
>> _GLIB_AUTO_FUNC_NAME(TypeName) (TypeName *_ptr) { if (*_ptr != none)
>> (func) (*_ptr); } \
>>          |                                                                                                         ^
>>    ../src/tests/unit/test-io-channel-command.c:38:19: note: ‘srcargv’ was declared here
>>       38 |     g_auto(GStrv) srcargv;
>>          |                   ^~~~~~~
>>    cc1: all warnings being treated as errors
>>
>> GCC version:
>>
>>    $ gcc --version
>>    gcc (GCC) 12.2.0
>>
>> Fixes: 68406d10859385c88da73d0106254a7f47e6652e ('tests/unit: cleanups for test-io-channel-command')
>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>> ---
>>   tests/unit/test-io-channel-command.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/tests/unit/test-io-channel-command.c b/tests/unit/test-io-channel-command.c
>> index 43e29c8cfb..ba0717d3c3 100644
>> --- a/tests/unit/test-io-channel-command.c
>> +++ b/tests/unit/test-io-channel-command.c
>> @@ -35,8 +35,8 @@ static void test_io_channel_command_fifo(bool async)
>>       g_autofree gchar *fifo = g_strdup_printf("%s/%s", tmpdir, TEST_FIFO);
>>       g_autoptr(GString) srcargs = g_string_new(socat);
>>       g_autoptr(GString) dstargs = g_string_new(socat);
>> -    g_auto(GStrv) srcargv;
>> -    g_auto(GStrv) dstargv;
>> +    g_auto(GStrv) srcargv = NULL;
>> +    g_auto(GStrv) dstargv = NULL;
>>       QIOChannel *src, *dst;
>>       QIOChannelTest *test;
> 
> Another approach would be to drop the GString usage which is premature
> and then we can allocate everything in order:

Yes, it looks like a better approach. I'm going to drop this patch from the trivial branch.

Could you send a patch?

Thanks,
Laurent

> 
> --8<---------------cut here---------------start------------->8---
> modified   tests/unit/test-io-channel-command.c
> @@ -33,19 +33,13 @@ static void test_io_channel_command_fifo(bool async)
>   {
>       g_autofree gchar *tmpdir = g_dir_make_tmp("qemu-test-io-channel.XXXXXX", NULL);
>       g_autofree gchar *fifo = g_strdup_printf("%s/%s", tmpdir, TEST_FIFO);
> -    g_autoptr(GString) srcargs = g_string_new(socat);
> -    g_autoptr(GString) dstargs = g_string_new(socat);
> -    g_auto(GStrv) srcargv;
> -    g_auto(GStrv) dstargv;
> +    g_autofree gchar *srcargs = g_strdup_printf("%s - PIPE:%s,wronly", socat, fifo);
> +    g_autofree gchar *dstargs = g_strdup_printf("%s PIPE:%s,rdonly -", socat, fifo);
> +    g_auto(GStrv) srcargv = g_strsplit(srcargs, " ", -1);
> +    g_auto(GStrv) dstargv = g_strsplit(dstargs, " ", -1);
>       QIOChannel *src, *dst;
>       QIOChannelTest *test;
>   
> -    g_string_append_printf(srcargs, " - PIPE:%s,wronly", fifo);
> -    g_string_append_printf(dstargs, " PIPE:%s,rdonly -", fifo);
> -
> -    srcargv = g_strsplit(srcargs->str, " ", -1);
> -    dstargv = g_strsplit(dstargs->str, " ", -1);
> -
>       src = QIO_CHANNEL(qio_channel_command_new_spawn((const char **) srcargv,
>                                                       O_WRONLY,
> --8<---------------cut here---------------end--------------->8---
> 
> 
> 
>