Re: [Qemu-devel] [Qemu-trivial] [PATCH] migration: Fix compiler warning ('caps' may be used uninitialized)

[Michael Tokarev <mjt@tls.msk.ru>]

29.09.2013 19:41, Stefan Weil wrote:
> The QEMU buildbot default_i386_debian_6_0 shows this warning:
>
>    CC    migration.o
> migration.c: In function 'qmp_query_migrate_capabilities':
> migration.c:149: warning:
>   'caps' may be used uninitialized in this function

Gah, how disgusting.  The code is correct, yet gcc complains
needlessly...

> While changing this code, I also replaced g_malloc0
> by the type safe g_new0.
>
> Signed-off-by: Stefan Weil <sw@weilnetz.de>
> ---
>
> Buildbot URL:
> http://buildbot.b1-systems.de/qemu/builders/default_i386_debian_6_0/builds/775/steps/compile/logs/stdio
>
>   migration.c |   12 +++++-------
>   1 file changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/migration.c b/migration.c
> index 200d404..8dcb6ce 100644
> --- a/migration.c
> +++ b/migration.c
> @@ -145,17 +145,15 @@ uint64_t migrate_max_downtime(void)
>
>   MigrationCapabilityStatusList *qmp_query_migrate_capabilities(Error **errp)
>   {
> -    MigrationCapabilityStatusList *head = NULL;
> -    MigrationCapabilityStatusList *caps;
> +    MigrationCapabilityStatusList *head =
> +        g_new0(MigrationCapabilityStatusList, 1);
> +    MigrationCapabilityStatusList *caps = head;
>       MigrationState *s = migrate_get_current();
>       int i;
>
>       for (i = 0; i < MIGRATION_CAPABILITY_MAX; i++) {
> -        if (head == NULL) {
> -            head = g_malloc0(sizeof(*caps));
> -            caps = head;
> -        } else {
> -            caps->next = g_malloc0(sizeof(*caps));
> +        if (i > 0) {
> +            caps->next = g_new0(MigrationCapabilityStatusList, i);
>               caps = caps->next;
>           }

But this assumes that MIGRATION_CAPABILITY_MAX > 0 ! ;)
Ie, that there's at least one entry (head) in the list.
Do we care?

Previous code was more natural, so to say...

I'd just initialize `caps' to the same NULL as `head'
initially...  Or maybe change for() into do..while()..
Dunno, somehow I don't like the new code much, either ;)

Thanks,

/mjt