Am 29.09.2013 22:13, schrieb Michael Tokarev: > 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...
That's not the first time where we help the compiler by modifying the code. > >> While changing this code, I also replaced g_malloc0 >> by the type safe g_new0. >> >> Signed-off-by: Stefan Weil <s...@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? I thought of MIGRATION_CAPABILITY_MAX == 0, too. In current QEMU it's defined in qapi-types.h: MIGRATION_CAPABILITY_MAX = 4, If there will be no MigrationCapability at all in the future, we should care about removing the code, not handle the 0 case. But of course we could add an assertion (feel free to add one, or I send an update). Should we fix the callers, too? Today, they assume that the function might return NULL (= no MigrationCapabilityexists). > > 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
