tests/qtest/migration-test: Use g_autofree to avoid leaks on error paths

Coverity notices that several places in the migration-test code fail
to free memory in error-exit paths.  This is pretty unimportant in
test case code, but we can avoid having to manually free the memory
entirely by using g_autofree.

The places where Coverity spotted a leak were relating to early exits
not freeing 'uri' in test_precopy_unix(), do_test_validate_uuid(),
migrate_postcopy_prepare() and test_migrate_auto_converge().  This
patch converts all the string-allocation in the test code to
g_autofree for consistency.

Fixes: Coverity CID 1432313, 1432315, 1432352, 1432364
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Message-Id: <20210506185819.9010-1-peter.maydell@linaro.org>
Signed-off-by: Thomas Huth <thuth@redhat.com>
This commit is contained in:
Peter Maydell 2021-05-06 19:58:19 +01:00 committed by Thomas Huth
parent e0447a834d
commit 2ed765fdee

View file

@ -110,13 +110,12 @@ static void init_bootfile(const char *bootpath, void *content, size_t len)
*/ */
static void wait_for_serial(const char *side) static void wait_for_serial(const char *side)
{ {
char *serialpath = g_strdup_printf("%s/%s", tmpfs, side); g_autofree char *serialpath = g_strdup_printf("%s/%s", tmpfs, side);
FILE *serialfile = fopen(serialpath, "r"); FILE *serialfile = fopen(serialpath, "r");
const char *arch = qtest_get_arch(); const char *arch = qtest_get_arch();
int started = (strcmp(side, "src_serial") == 0 && int started = (strcmp(side, "src_serial") == 0 &&
strcmp(arch, "ppc64") == 0) ? 0 : 1; strcmp(arch, "ppc64") == 0) ? 0 : 1;
g_free(serialpath);
do { do {
int readvalue = fgetc(serialfile); int readvalue = fgetc(serialfile);
@ -274,10 +273,9 @@ static void check_guests_ram(QTestState *who)
static void cleanup(const char *filename) static void cleanup(const char *filename)
{ {
char *path = g_strdup_printf("%s/%s", tmpfs, filename); g_autofree char *path = g_strdup_printf("%s/%s", tmpfs, filename);
unlink(path); unlink(path);
g_free(path);
} }
static char *SocketAddress_to_str(SocketAddress *addr) static char *SocketAddress_to_str(SocketAddress *addr)
@ -374,11 +372,8 @@ static char *migrate_get_parameter_str(QTestState *who,
static void migrate_check_parameter_str(QTestState *who, const char *parameter, static void migrate_check_parameter_str(QTestState *who, const char *parameter,
const char *value) const char *value)
{ {
char *result; g_autofree char *result = migrate_get_parameter_str(who, parameter);
result = migrate_get_parameter_str(who, parameter);
g_assert_cmpstr(result, ==, value); g_assert_cmpstr(result, ==, value);
g_free(result);
} }
static void migrate_set_parameter_str(QTestState *who, const char *parameter, static void migrate_set_parameter_str(QTestState *who, const char *parameter,
@ -495,12 +490,14 @@ static void migrate_start_destroy(MigrateStart *args)
static int test_migrate_start(QTestState **from, QTestState **to, static int test_migrate_start(QTestState **from, QTestState **to,
const char *uri, MigrateStart *args) const char *uri, MigrateStart *args)
{ {
gchar *arch_source, *arch_target; g_autofree gchar *arch_source = NULL;
gchar *cmd_source, *cmd_target; g_autofree gchar *arch_target = NULL;
g_autofree gchar *cmd_source = NULL;
g_autofree gchar *cmd_target = NULL;
const gchar *ignore_stderr; const gchar *ignore_stderr;
char *bootpath = NULL; g_autofree char *bootpath = NULL;
char *shmem_opts; g_autofree char *shmem_opts = NULL;
char *shmem_path; g_autofree char *shmem_path = NULL;
const char *arch = qtest_get_arch(); const char *arch = qtest_get_arch();
const char *machine_opts = NULL; const char *machine_opts = NULL;
const char *memory_size; const char *memory_size;
@ -559,8 +556,6 @@ static int test_migrate_start(QTestState **from, QTestState **to,
g_assert_not_reached(); g_assert_not_reached();
} }
g_free(bootpath);
if (!getenv("QTEST_LOG") && args->hide_stderr) { if (!getenv("QTEST_LOG") && args->hide_stderr) {
ignore_stderr = "2>/dev/null"; ignore_stderr = "2>/dev/null";
} else { } else {
@ -588,11 +583,9 @@ static int test_migrate_start(QTestState **from, QTestState **to,
memory_size, tmpfs, memory_size, tmpfs,
arch_source, shmem_opts, args->opts_source, arch_source, shmem_opts, args->opts_source,
ignore_stderr); ignore_stderr);
g_free(arch_source);
if (!args->only_target) { if (!args->only_target) {
*from = qtest_init(cmd_source); *from = qtest_init(cmd_source);
} }
g_free(cmd_source);
cmd_target = g_strdup_printf("-accel kvm -accel tcg%s%s " cmd_target = g_strdup_printf("-accel kvm -accel tcg%s%s "
"-name target,debug-threads=on " "-name target,debug-threads=on "
@ -605,18 +598,14 @@ static int test_migrate_start(QTestState **from, QTestState **to,
memory_size, tmpfs, uri, memory_size, tmpfs, uri,
arch_target, shmem_opts, arch_target, shmem_opts,
args->opts_target, ignore_stderr); args->opts_target, ignore_stderr);
g_free(arch_target);
*to = qtest_init(cmd_target); *to = qtest_init(cmd_target);
g_free(cmd_target);
g_free(shmem_opts);
/* /*
* Remove shmem file immediately to avoid memory leak in test failed case. * Remove shmem file immediately to avoid memory leak in test failed case.
* It's valid becase QEMU has already opened this file * It's valid becase QEMU has already opened this file
*/ */
if (args->use_shmem) { if (args->use_shmem) {
unlink(shmem_path); unlink(shmem_path);
g_free(shmem_path);
} }
out: out:
@ -662,7 +651,7 @@ static int migrate_postcopy_prepare(QTestState **from_ptr,
QTestState **to_ptr, QTestState **to_ptr,
MigrateStart *args) MigrateStart *args)
{ {
char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs); g_autofree char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
QTestState *from, *to; QTestState *from, *to;
if (test_migrate_start(&from, &to, uri, args)) { if (test_migrate_start(&from, &to, uri, args)) {
@ -684,7 +673,6 @@ static int migrate_postcopy_prepare(QTestState **from_ptr,
wait_for_serial("src_serial"); wait_for_serial("src_serial");
migrate_qmp(from, uri, "{}"); migrate_qmp(from, uri, "{}");
g_free(uri);
wait_for_migration_pass(from); wait_for_migration_pass(from);
@ -724,7 +712,7 @@ static void test_postcopy_recovery(void)
{ {
MigrateStart *args = migrate_start_new(); MigrateStart *args = migrate_start_new();
QTestState *from, *to; QTestState *from, *to;
char *uri; g_autofree char *uri = NULL;
args->hide_stderr = true; args->hide_stderr = true;
@ -775,7 +763,6 @@ static void test_postcopy_recovery(void)
(const char * []) { "failed", "active", (const char * []) { "failed", "active",
"completed", NULL }); "completed", NULL });
migrate_qmp(from, uri, "{'resume': true}"); migrate_qmp(from, uri, "{'resume': true}");
g_free(uri);
/* Restore the postcopy bandwidth to unlimited */ /* Restore the postcopy bandwidth to unlimited */
migrate_set_parameter_int(from, "max-postcopy-bandwidth", 0); migrate_set_parameter_int(from, "max-postcopy-bandwidth", 0);
@ -800,7 +787,7 @@ static void test_baddest(void)
static void test_precopy_unix(void) static void test_precopy_unix(void)
{ {
char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs); g_autofree char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
MigrateStart *args = migrate_start_new(); MigrateStart *args = migrate_start_new();
QTestState *from, *to; QTestState *from, *to;
@ -836,14 +823,13 @@ static void test_precopy_unix(void)
wait_for_migration_complete(from); wait_for_migration_complete(from);
test_migrate_end(from, to, true); test_migrate_end(from, to, true);
g_free(uri);
} }
#if 0 #if 0
/* Currently upset on aarch64 TCG */ /* Currently upset on aarch64 TCG */
static void test_ignore_shared(void) static void test_ignore_shared(void)
{ {
char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs); g_autofree char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
QTestState *from, *to; QTestState *from, *to;
if (test_migrate_start(&from, &to, uri, false, true, NULL, NULL)) { if (test_migrate_start(&from, &to, uri, false, true, NULL, NULL)) {
@ -873,7 +859,6 @@ static void test_ignore_shared(void)
g_assert_cmpint(read_ram_property_int(from, "transferred"), <, 1024 * 1024); g_assert_cmpint(read_ram_property_int(from, "transferred"), <, 1024 * 1024);
test_migrate_end(from, to, true); test_migrate_end(from, to, true);
g_free(uri);
} }
#endif #endif
@ -925,16 +910,15 @@ static void test_xbzrle(const char *uri)
static void test_xbzrle_unix(void) static void test_xbzrle_unix(void)
{ {
char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs); g_autofree char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
test_xbzrle(uri); test_xbzrle(uri);
g_free(uri);
} }
static void test_precopy_tcp(void) static void test_precopy_tcp(void)
{ {
MigrateStart *args = migrate_start_new(); MigrateStart *args = migrate_start_new();
char *uri; g_autofree char *uri = NULL;
QTestState *from, *to; QTestState *from, *to;
if (test_migrate_start(&from, &to, "tcp:127.0.0.1:0", args)) { if (test_migrate_start(&from, &to, "tcp:127.0.0.1:0", args)) {
@ -971,7 +955,6 @@ static void test_precopy_tcp(void)
wait_for_migration_complete(from); wait_for_migration_complete(from);
test_migrate_end(from, to, true); test_migrate_end(from, to, true);
g_free(uri);
} }
static void test_migrate_fd_proto(void) static void test_migrate_fd_proto(void)
@ -1060,7 +1043,7 @@ static void test_migrate_fd_proto(void)
static void do_test_validate_uuid(MigrateStart *args, bool should_fail) static void do_test_validate_uuid(MigrateStart *args, bool should_fail)
{ {
char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs); g_autofree char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
QTestState *from, *to; QTestState *from, *to;
if (test_migrate_start(&from, &to, uri, args)) { if (test_migrate_start(&from, &to, uri, args)) {
@ -1088,7 +1071,6 @@ static void do_test_validate_uuid(MigrateStart *args, bool should_fail)
} }
test_migrate_end(from, to, false); test_migrate_end(from, to, false);
g_free(uri);
} }
static void test_validate_uuid(void) static void test_validate_uuid(void)
@ -1136,7 +1118,7 @@ static void test_validate_uuid_dst_not_set(void)
static void test_migrate_auto_converge(void) static void test_migrate_auto_converge(void)
{ {
char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs); g_autofree char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
MigrateStart *args = migrate_start_new(); MigrateStart *args = migrate_start_new();
QTestState *from, *to; QTestState *from, *to;
int64_t remaining, percentage; int64_t remaining, percentage;
@ -1214,7 +1196,6 @@ static void test_migrate_auto_converge(void)
wait_for_serial("dest_serial"); wait_for_serial("dest_serial");
wait_for_migration_complete(from); wait_for_migration_complete(from);
g_free(uri);
test_migrate_end(from, to, true); test_migrate_end(from, to, true);
} }
@ -1224,7 +1205,7 @@ static void test_multifd_tcp(const char *method)
MigrateStart *args = migrate_start_new(); MigrateStart *args = migrate_start_new();
QTestState *from, *to; QTestState *from, *to;
QDict *rsp; QDict *rsp;
char *uri; g_autofree char *uri = NULL;
if (test_migrate_start(&from, &to, "defer", args)) { if (test_migrate_start(&from, &to, "defer", args)) {
return; return;
@ -1273,7 +1254,6 @@ static void test_multifd_tcp(const char *method)
wait_for_serial("dest_serial"); wait_for_serial("dest_serial");
wait_for_migration_complete(from); wait_for_migration_complete(from);
test_migrate_end(from, to, true); test_migrate_end(from, to, true);
g_free(uri);
} }
static void test_multifd_tcp_none(void) static void test_multifd_tcp_none(void)
@ -1309,7 +1289,7 @@ static void test_multifd_tcp_cancel(void)
MigrateStart *args = migrate_start_new(); MigrateStart *args = migrate_start_new();
QTestState *from, *to, *to2; QTestState *from, *to, *to2;
QDict *rsp; QDict *rsp;
char *uri; g_autofree char *uri = NULL;
args->hide_stderr = true; args->hide_stderr = true;
@ -1387,7 +1367,6 @@ static void test_multifd_tcp_cancel(void)
wait_for_serial("dest_serial"); wait_for_serial("dest_serial");
wait_for_migration_complete(from); wait_for_migration_complete(from);
test_migrate_end(from, to2, true); test_migrate_end(from, to2, true);
g_free(uri);
} }
int main(int argc, char **argv) int main(int argc, char **argv)