From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:39216)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <sw@weilnetz.de>) id 1VRSyL-0003Jf-55
	for qemu-devel@nongnu.org; Wed, 02 Oct 2013 16:24:44 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <sw@weilnetz.de>) id 1VRSyB-0004BO-K5
	for qemu-devel@nongnu.org; Wed, 02 Oct 2013 16:24:36 -0400
Message-ID: <524C80ED.3020507@weilnetz.de>
Date: Wed, 02 Oct 2013 22:24:13 +0200
From: Stefan Weil <sw@weilnetz.de>
MIME-Version: 1.0
References: <1380469272-19230-1-git-send-email-sw@weilnetz.de>	<52488A00.6000405@msgid.tls.msk.ru>
	<52488E98.3060505@weilnetz.de>
	<871u46iq96.fsf@blackfin.pond.sub.org>
	<5249E4CD.1050605@weilnetz.de> <524C6DD0.9070605@msgid.tls.msk.ru>
In-Reply-To: <524C6DD0.9070605@msgid.tls.msk.ru>
Content-Type: text/plain; charset=ISO-8859-15
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [Qemu-trivial] [PATCH] migration: Fix compiler
 warning ('caps' may be used uninitialized)
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Michael Tokarev <mjt@tls.msk.ru>
Cc: qemu-trivial <qemu-trivial@nongnu.org>, Paolo Bonzini <pbonzini@redhat.com>, Markus Armbruster <armbru@redhat.com>, qemu-devel <qemu-devel@nongnu.org>

Am 02.10.2013 21:02, schrieb Michael Tokarev:
> How about this:
>
> diff --git a/migration.c b/migration.c
> index b4f8462..6066ab4 100644
> --- a/migration.c
> +++ b/migration.c
> @@ -146,22 +146,16 @@ uint64_t migrate_max_downtime(void)
>  MigrationCapabilityStatusList *qmp_query_migrate_capabilities(Error
> **errp)
>  {
>      MigrationCapabilityStatusList *head = NULL;
> -    MigrationCapabilityStatusList *caps;
> +    MigrationCapabilityStatusList **capp = &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));
> -            caps = caps->next;
> -        }
> -        caps->value =
> -            g_malloc(sizeof(*caps->value));
> -        caps->value->capability = i;
> -        caps->value->state = s->enabled_capabilities[i];
> +        *capp = g_malloc0(sizeof(*head));
> +        (*capp)->value = g_malloc(sizeof(*head->value));
> +        (*capp)->value->capability = i;
> +        (*capp)->value->state = s->enabled_capabilities[i];
> +       capp = &(*capp)->next;
>      }
>
>      return head;
>
>
> This is what I had in mind at the very beginnig, but only now tried
> to make a patch...
>
> Thanks,
>
> /mjt
> Thanks,


That's a possible solution. Paolo also sent a sketch of a similar
solution. And here are two more:

MigrationCapabilityStatusList *qmp_query_migrate_capabilities(Error **errp)
{
    MigrationCapabilityStatusList *head = NULL;
    MigrationState *s = migrate_get_current();
    MigrationCapability i = MIGRATION_CAPABILITY_MAX;

    do {
        MigrationCapabilityStatusList *caps =
            g_new(MigrationCapabilityStatusList, 1);
        i--;
        caps->next = head;
        caps->value = g_new(MigrationCapabilityStatus, 1);
        caps->value->capability = i;
        caps->value->state = s->enabled_capabilities[i];
        head = caps;
    } while (i > 0);

    return head;
}

MigrationCapabilityStatusList *qmp_query_migrate_capabilities(Error **errp)
{
    MigrationCapabilityStatusList *head = NULL;
    MigrationCapabilityStatusList *prev = NULL;
    MigrationState *s = migrate_get_current();
    MigrationCapability i;

    for (i = 0; i < MIGRATION_CAPABILITY_MAX; i++) {
        MigrationCapabilityStatusList *caps =
            g_new(MigrationCapabilityStatusList, 1);
        if (prev == NULL) {
            head = caps;
        } else {
            prev->next = caps;
            prev = caps;
        }
        caps->value = g_new(MigrationCapabilityStatus, 1);
        caps->value->capability = i;
        caps->value->state = s->enabled_capabilities[i];
    }

    return head;
}

Which one do we take? Any correct solution which fixes the compiler
warning is fine for me (although I prefer g_new instead of g_malloc as
you might have guessed). :-)

Regards,
Stefan