[Qemu-devel] [PATCH for-2.7 v4 00/36] Various memory leak fixes

[Qemu-devel] [PATCH for-2.7 v4 00/36] Various memory leak fixes

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Hi,

Thanks to AddressSanitizer (ASAN), I found a number of direct leaks
worth fixing. Note that there are probably many indirect leaks left (I
am adding some here), I haven't investigated much yet.

There are still a number of direct leaks remaining, in particular in
the tests, but my libc doesn't give me good backtraces.

In order to easily switch to asan-enabled build, I make use of make
CFLAGS argument, which is why the first patch is also there.

v4:
- document qtest_add_data_func_full seperately
- add g_(s)list_free_full in glib-compat
- simplify tcp_chr_close()
- move dma_buf_commit() down before the early return case
- dropped a qdist leak patch already applied

v3:
- add glib<2.34 fallback for g_test_add_data_func_full() using
  g_test_add_vtable() with an explicit comment
- add and use a tcp_chr_free_connection(), from tcp_chr_disconnect()
  (suggested by Paolo)
- remove bad comment from "free MuxDriver" patch (Eric)
- replace GFunc casts with a qemu_g_func_free() inline (suggested by Eric)
- replace CFLAGS override patch with a toplevel CFLAGS filter
  (suggested by Paolo)
- drop a patch already merged upstream
- update reviewed-by tags (28/36)

v2:
- split "pci-bus: do not allocate and leak bsel" as a seperate patch,
  since the direct leak looks like a bug
- hook virtio_input_finalize to the abstract base class
- use mkdtemp in test-io-channel-command
- fix pc-cpu-test commit message
- move the sglist destruction inside of ncq_err, call dma_buf_commit
  in ide_dma_cb, as suggested by John Snow
- fix build with glib 2.22
- add the given reviewed-by tags (22/37, 15 left)

Marc-André Lureau (36):
  build-sys: fix building with make CFLAGS=.. argument
  tests: fix test-qga leaks
  glib-compat: add g_(s)list_free_full()
  qga: free the whole blacklist
  qga: free remaining leaking state
  tests: fix test-cutils leaks
  tests: fix test-vmstate leaks
  tests: fix test-iov leaks
  tests: fix check-qom-interface leaks
  tests: fix check-qom-proplist leaks
  tests: fix small leak in test-io-channel-command
  tests: fix leak in test-string-input-visitor
  portio: keep references on portio
  numa: do not leak NumaOptions
  pc: simplify passing qemu_irq
  pc: don't leak a20_line
  machine: use class base init generated name
  acpi-build: fix array leak
  char: free the tcp connection data when closing
  char: free MuxDriver when closing
  tests: fix qom-test leaks
  pc: free i8259
  pc: keep gsi reference
  ahci: free irqs array
  sd: free timer
  qjson: free str
  virtio-input: free config list
  ipmi: free extern timer
  usb: free USBDevice.strings
  usb: free leaking path
  bus: simplify name handling
  tests: add qtest_add_data_func_full
  tests: pc-cpu-test leaks fixes
  tests: fix rsp leak in postcopy-test
  ahci: fix sglist leak on retry
  tests: fix postcopy-test leaks

 Makefile                          |  3 +-
 hw/audio/gus.c                    |  9 ++++--
 hw/audio/sb16.c                   |  4 ++-
 hw/block/fdc.c                    |  4 ++-
 hw/char/parallel.c                |  3 +-
 hw/core/bus.c                     | 21 ++++----------
 hw/core/machine.c                 |  1 +
 hw/display/vga-isa.c              |  8 ++++--
 hw/dma/i8257.c                    |  6 ++--
 hw/i386/acpi-build.c              |  4 +--
 hw/i386/pc.c                      |  9 +++---
 hw/i386/pc_piix.c                 | 17 ++++++------
 hw/i386/pc_q35.c                  | 15 +++++-----
 hw/ide/ahci.c                     |  4 +--
 hw/ide/core.c                     |  7 +++--
 hw/input/pckbd.c                  |  4 +--
 hw/input/virtio-input.c           | 11 ++++++++
 hw/ipmi/ipmi_bmc_extern.c         |  9 ++++++
 hw/isa/isa-bus.c                  | 14 ++++------
 hw/sd/sd.c                        |  9 ++++++
 hw/usb/bus.c                      |  7 +++++
 hw/usb/desc.c                     |  1 +
 include/glib-compat.h             | 24 ++++++++++++++++
 include/hw/boards.h               |  2 +-
 include/hw/i386/pc.h              |  4 +--
 include/hw/ide/internal.h         |  2 ++
 include/hw/isa/i8257.h            |  2 ++
 include/hw/isa/isa.h              |  5 +++-
 include/hw/qdev-core.h            |  2 +-
 migration/qjson.c                 |  1 +
 numa.c                            | 15 +++++-----
 pc-bios/optionrom/Makefile        |  2 --
 qemu-char.c                       | 58 ++++++++++++++++++++++++++-------------
 qga/guest-agent-command-state.c   |  6 ++++
 qga/guest-agent-core.h            |  1 +
 qga/main.c                        | 13 +++++----
 tests/check-qom-interface.c       |  1 +
 tests/check-qom-proplist.c        | 16 +++++++++++
 tests/libqtest.c                  | 15 ++++++++++
 tests/libqtest.h                  | 17 ++++++++++++
 tests/pc-cpu-test.c               | 24 ++++++++++++----
 tests/postcopy-test.c             |  7 +++--
 tests/qom-test.c                  |  5 ++--
 tests/test-cutils.c               | 24 ++++++++++------
 tests/test-io-channel-command.c   | 20 +++++++++-----
 tests/test-iov.c                  |  7 +++++
 tests/test-qga.c                  |  8 +++++-
 tests/test-string-input-visitor.c |  1 +
 tests/test-vmstate.c              |  8 ++++--
 49 files changed, 331 insertions(+), 129 deletions(-)

-- 
2.9.0

[Qemu-devel] [PATCH for-2.7 v4 01/36] build-sys: fix building with make CFLAGS=.. argument

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

When calling make with a CFLAGS=.. argument, the -g/-O filter is not
applied, which may result with build failure with ASAN for example. It
could be solved with an 'override' directive on CFLAGS, but that would
actually prevent setting different CFLAGS manually.

Instead, filter the CFLAGS argument from the top-level Makefile (so
you could still call make with a different CFLAGS argument on a
rom/Makefile manually)

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
 Makefile                   | 3 ++-
 pc-bios/optionrom/Makefile | 2 --
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/Makefile b/Makefile
index 0d7647f..50b4b3a 100644
--- a/Makefile
+++ b/Makefile
@@ -225,8 +225,9 @@ dtc/%:
 $(SUBDIR_RULES): libqemuutil.a libqemustub.a $(common-obj-y) $(qom-obj-y) $(crypto-aes-obj-$(CONFIG_USER_ONLY))
 
 ROMSUBDIR_RULES=$(patsubst %,romsubdir-%, $(ROMS))
+# Only keep -O and -g cflags
 romsubdir-%:
-	$(call quiet-command,$(MAKE) $(SUBDIR_MAKEFLAGS) -C pc-bios/$* V="$(V)" TARGET_DIR="$*/",)
+	$(call quiet-command,$(MAKE) $(SUBDIR_MAKEFLAGS) -C pc-bios/$* V="$(V)" TARGET_DIR="$*/" CFLAGS="$(filter -O% -g%,$(CFLAGS))",)
 
 ALL_SUBDIRS=$(TARGET_DIRS) $(patsubst %,pc-bios/%, $(ROMS))
 
diff --git a/pc-bios/optionrom/Makefile b/pc-bios/optionrom/Makefile
index 24e175e..6bab490 100644
--- a/pc-bios/optionrom/Makefile
+++ b/pc-bios/optionrom/Makefile
@@ -24,8 +24,6 @@ QEMU_CFLAGS += $(call cc-option, $(QEMU_CFLAGS), -no-integrated-as)
 QEMU_CFLAGS += -m32 -include $(SRC_PATH)/pc-bios/optionrom/code16gcc.h
 endif
 
-# Drop gcov and glib flags
-CFLAGS := $(filter -O% -g%, $(CFLAGS))
 QEMU_INCLUDES += -I$(SRC_PATH)
 
 Wa = -Wa,
-- 
2.9.0

[Qemu-devel] [PATCH for-2.7 v4 02/36] tests: fix test-qga leaks

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 tests/test-qga.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/tests/test-qga.c b/tests/test-qga.c
index dac8fb8..21f44f8 100644
--- a/tests/test-qga.c
+++ b/tests/test-qga.c
@@ -398,6 +398,7 @@ static void test_qga_file_ops(gconstpointer fix)
     /* check content */
     path = g_build_filename(fixture->test_dir, "foo", NULL);
     f = fopen(path, "r");
+    g_free(path);
     g_assert_nonnull(f);
     count = fread(tmp, 1, sizeof(tmp), f);
     g_assert_cmpint(count, ==, sizeof(helloworld));
@@ -700,7 +701,9 @@ static void test_qga_config(gconstpointer data)
     cwd = g_get_current_dir();
     cmd = g_strdup_printf("%s%cqemu-ga -D",
                           cwd, G_DIR_SEPARATOR);
+    g_free(cwd);
     g_shell_parse_argv(cmd, NULL, &argv, &error);
+    g_free(cmd);
     g_assert_no_error(error);
 
     env[0] = g_strdup_printf("QGA_CONF=tests%cdata%ctest-qga-config",
@@ -708,6 +711,8 @@ static void test_qga_config(gconstpointer data)
     env[1] = NULL;
     g_spawn_sync(NULL, argv, env, 0,
                  NULL, NULL, &out, &err, &status, &error);
+    g_strfreev(argv);
+
     g_assert_no_error(error);
     g_assert_cmpstr(err, ==, "");
     g_assert_cmpint(status, ==, 0);
-- 
2.9.0

[Qemu-devel] [PATCH for-2.7 v4 03/36] glib-compat: add g_(s)list_free_full()

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Those functions are only available since glib 2.28.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 include/glib-compat.h | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/include/glib-compat.h b/include/glib-compat.h
index 01aa7b3..ff7eae5 100644
--- a/include/glib-compat.h
+++ b/include/glib-compat.h
@@ -260,4 +260,28 @@ static inline void g_hash_table_add(GHashTable *hash_table, gpointer key)
     } while (0)
 #endif
 
+#if !GLIB_CHECK_VERSION(2, 28, 0)
+static inline void g_list_free_full(GList *list, GDestroyNotify free_func)
+{
+    GList *l;
+
+    for (l = list; l; l = l->next) {
+        free_func(l->data);
+    }
+
+    g_list_free(list);
+}
+
+static inline void g_slist_free_full(GSList *list, GDestroyNotify free_func)
+{
+    GSList *l;
+
+    for (l = list; l; l = l->next) {
+        free_func(l->data);
+    }
+
+    g_slist_free(list);
+}
+#endif
+
 #endif
-- 
2.9.0

[Qemu-devel] [PATCH for-2.7 v4 04/36] qga: free the whole blacklist

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Free the config blacklist list, not just the elements. Do it so in the
more appropriate function config_free().

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 qga/main.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/qga/main.c b/qga/main.c
index 4c3b2c7..bb48214 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -1175,6 +1175,7 @@ static void config_free(GAConfig *config)
 #ifdef CONFIG_FSFREEZE
     g_free(config->fsfreeze_hook);
 #endif
+    g_list_free_full(config->blacklist, g_free);
     g_free(config);
 }
 
@@ -1310,11 +1311,6 @@ static int run_agent(GAState *s, GAConfig *config)
     return EXIT_SUCCESS;
 }
 
-static void free_blacklist_entry(gpointer entry, gpointer unused)
-{
-    g_free(entry);
-}
-
 int main(int argc, char **argv)
 {
     int ret = EXIT_SUCCESS;
@@ -1379,7 +1375,6 @@ end:
     if (s->channel) {
         ga_channel_free(s->channel);
     }
-    g_list_foreach(config->blacklist, free_blacklist_entry, NULL);
     g_free(s->pstate_filepath);
     g_free(s->state_filepath_isfrozen);
 
-- 
2.9.0

[Qemu-devel] [PATCH for-2.7 v4 05/36] qga: free remaining leaking state

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 qga/guest-agent-command-state.c | 6 ++++++
 qga/guest-agent-core.h          | 1 +
 qga/main.c                      | 6 ++++++
 3 files changed, 13 insertions(+)

diff --git a/qga/guest-agent-command-state.c b/qga/guest-agent-command-state.c
index 4de229c..e609d32 100644
--- a/qga/guest-agent-command-state.c
+++ b/qga/guest-agent-command-state.c
@@ -71,3 +71,9 @@ GACommandState *ga_command_state_new(void)
     cs->groups = NULL;
     return cs;
 }
+
+void ga_command_state_free(GACommandState *cs)
+{
+    g_slist_free_full(cs->groups, g_free);
+    g_free(cs);
+}
diff --git a/qga/guest-agent-core.h b/qga/guest-agent-core.h
index 0a49516..63e9d39 100644
--- a/qga/guest-agent-core.h
+++ b/qga/guest-agent-core.h
@@ -28,6 +28,7 @@ void ga_command_state_add(GACommandState *cs,
 void ga_command_state_init_all(GACommandState *cs);
 void ga_command_state_cleanup_all(GACommandState *cs);
 GACommandState *ga_command_state_new(void);
+void ga_command_state_free(GACommandState *cs);
 bool ga_logging_enabled(GAState *s);
 void ga_disable_logging(GAState *s);
 void ga_enable_logging(GAState *s);
diff --git a/qga/main.c b/qga/main.c
index bb48214..0b9d04e 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -1371,6 +1371,8 @@ int main(int argc, char **argv)
 end:
     if (s->command_state) {
         ga_command_state_cleanup_all(s->command_state);
+        ga_command_state_free(s->command_state);
+        json_message_parser_destroy(&s->parser);
     }
     if (s->channel) {
         ga_channel_free(s->channel);
@@ -1383,6 +1385,10 @@ end:
     }
 
     config_free(config);
+    if (s->main_loop) {
+        g_main_loop_unref(s->main_loop);
+    }
+    g_free(s);
 
     return ret;
 }
-- 
2.9.0

[Qemu-devel] [PATCH for-2.7 v4 06/36] tests: fix test-cutils leaks

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Spotted thanks to ASAN.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 tests/test-cutils.c | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/tests/test-cutils.c b/tests/test-cutils.c
index 64e3e95..20b0f59 100644
--- a/tests/test-cutils.c
+++ b/tests/test-cutils.c
@@ -378,7 +378,7 @@ static void test_qemu_strtol_hex(void)
 
 static void test_qemu_strtol_max(void)
 {
-    const char *str = g_strdup_printf("%ld", LONG_MAX);
+    char *str = g_strdup_printf("%ld", LONG_MAX);
     char f = 'X';
     const char *endptr = &f;
     long res = 999;
@@ -389,6 +389,7 @@ static void test_qemu_strtol_max(void)
     g_assert_cmpint(err, ==, 0);
     g_assert_cmpint(res, ==, LONG_MAX);
     g_assert(endptr == str + strlen(str));
+    g_free(str);
 }
 
 static void test_qemu_strtol_overflow(void)
@@ -497,7 +498,7 @@ static void test_qemu_strtol_full_trailing(void)
 
 static void test_qemu_strtol_full_max(void)
 {
-    const char *str = g_strdup_printf("%ld", LONG_MAX);
+    char *str = g_strdup_printf("%ld", LONG_MAX);
     long res;
     int err;
 
@@ -505,6 +506,7 @@ static void test_qemu_strtol_full_max(void)
 
     g_assert_cmpint(err, ==, 0);
     g_assert_cmpint(res, ==, LONG_MAX);
+    g_free(str);
 }
 
 static void test_qemu_strtoul_correct(void)
@@ -662,7 +664,7 @@ static void test_qemu_strtoul_hex(void)
 
 static void test_qemu_strtoul_max(void)
 {
-    const char *str = g_strdup_printf("%lu", ULONG_MAX);
+    char *str = g_strdup_printf("%lu", ULONG_MAX);
     char f = 'X';
     const char *endptr = &f;
     unsigned long res = 999;
@@ -673,6 +675,7 @@ static void test_qemu_strtoul_max(void)
     g_assert_cmpint(err, ==, 0);
     g_assert_cmpint(res, ==, ULONG_MAX);
     g_assert(endptr == str + strlen(str));
+    g_free(str);
 }
 
 static void test_qemu_strtoul_overflow(void)
@@ -776,7 +779,7 @@ static void test_qemu_strtoul_full_trailing(void)
 
 static void test_qemu_strtoul_full_max(void)
 {
-    const char *str = g_strdup_printf("%lu", ULONG_MAX);
+    char *str = g_strdup_printf("%lu", ULONG_MAX);
     unsigned long res = 999;
     int err;
 
@@ -784,6 +787,7 @@ static void test_qemu_strtoul_full_max(void)
 
     g_assert_cmpint(err, ==, 0);
     g_assert_cmpint(res, ==, ULONG_MAX);
+    g_free(str);
 }
 
 static void test_qemu_strtoll_correct(void)
@@ -941,7 +945,7 @@ static void test_qemu_strtoll_hex(void)
 
 static void test_qemu_strtoll_max(void)
 {
-    const char *str = g_strdup_printf("%lld", LLONG_MAX);
+    char *str = g_strdup_printf("%lld", LLONG_MAX);
     char f = 'X';
     const char *endptr = &f;
     int64_t res = 999;
@@ -952,6 +956,7 @@ static void test_qemu_strtoll_max(void)
     g_assert_cmpint(err, ==, 0);
     g_assert_cmpint(res, ==, LLONG_MAX);
     g_assert(endptr == str + strlen(str));
+    g_free(str);
 }
 
 static void test_qemu_strtoll_overflow(void)
@@ -1058,7 +1063,7 @@ static void test_qemu_strtoll_full_trailing(void)
 static void test_qemu_strtoll_full_max(void)
 {
 
-    const char *str = g_strdup_printf("%lld", LLONG_MAX);
+    char *str = g_strdup_printf("%lld", LLONG_MAX);
     int64_t res;
     int err;
 
@@ -1066,6 +1071,7 @@ static void test_qemu_strtoll_full_max(void)
 
     g_assert_cmpint(err, ==, 0);
     g_assert_cmpint(res, ==, LLONG_MAX);
+    g_free(str);
 }
 
 static void test_qemu_strtoull_correct(void)
@@ -1223,7 +1229,7 @@ static void test_qemu_strtoull_hex(void)
 
 static void test_qemu_strtoull_max(void)
 {
-    const char *str = g_strdup_printf("%llu", ULLONG_MAX);
+    char *str = g_strdup_printf("%llu", ULLONG_MAX);
     char f = 'X';
     const char *endptr = &f;
     uint64_t res = 999;
@@ -1234,6 +1240,7 @@ static void test_qemu_strtoull_max(void)
     g_assert_cmpint(err, ==, 0);
     g_assert_cmpint(res, ==, ULLONG_MAX);
     g_assert(endptr == str + strlen(str));
+    g_free(str);
 }
 
 static void test_qemu_strtoull_overflow(void)
@@ -1339,7 +1346,7 @@ static void test_qemu_strtoull_full_trailing(void)
 
 static void test_qemu_strtoull_full_max(void)
 {
-    const char *str = g_strdup_printf("%lld", ULLONG_MAX);
+    char *str = g_strdup_printf("%lld", ULLONG_MAX);
     uint64_t res = 999;
     int err;
 
@@ -1347,6 +1354,7 @@ static void test_qemu_strtoull_full_max(void)
 
     g_assert_cmpint(err, ==, 0);
     g_assert_cmpint(res, ==, ULLONG_MAX);
+    g_free(str);
 }
 
 static void test_qemu_strtosz_simple(void)
-- 
2.9.0

[Qemu-devel] [PATCH for-2.7 v4 07/36] tests: fix test-vmstate leaks

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Spotted thanks to ASAN.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 tests/test-vmstate.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/tests/test-vmstate.c b/tests/test-vmstate.c
index 41fd841..d8da26f 100644
--- a/tests/test-vmstate.c
+++ b/tests/test-vmstate.c
@@ -50,16 +50,20 @@ static QEMUFile *open_test_file(bool write)
 {
     int fd = dup(temp_fd);
     QIOChannel *ioc;
+    QEMUFile *f;
+
     lseek(fd, 0, SEEK_SET);
     if (write) {
         g_assert_cmpint(ftruncate(fd, 0), ==, 0);
     }
     ioc = QIO_CHANNEL(qio_channel_file_new_fd(fd));
     if (write) {
-        return qemu_fopen_channel_output(ioc);
+        f = qemu_fopen_channel_output(ioc);
     } else {
-        return qemu_fopen_channel_input(ioc);
+        f = qemu_fopen_channel_input(ioc);
     }
+    object_unref(OBJECT(ioc));
+    return f;
 }
 
 #define SUCCESS(val) \
-- 
2.9.0

[Qemu-devel] [PATCH for-2.7 v4 08/36] tests: fix test-iov leaks

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Spotted thanks to ASAN.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 tests/test-iov.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/tests/test-iov.c b/tests/test-iov.c
index 46ae25e..a22d71f 100644
--- a/tests/test-iov.c
+++ b/tests/test-iov.c
@@ -208,6 +208,9 @@ static void test_io(void)
                } while(k < j);
            }
        }
+       iov_free(iov, niov);
+       g_free(buf);
+       g_free(siov);
        exit(0);
 
     } else {
@@ -246,6 +249,10 @@ static void test_io(void)
                test_iov_bytes(iov, niov, i, j - i);
            }
         }
+
+       iov_free(iov, niov);
+       g_free(buf);
+       g_free(siov);
      }
 #endif
 }
-- 
2.9.0

[Qemu-devel] [PATCH for-2.7 v4 09/36] tests: fix check-qom-interface leaks

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Found thanks to ASAN.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 tests/check-qom-interface.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/check-qom-interface.c b/tests/check-qom-interface.c
index 719ddcf..f87c9aa 100644
--- a/tests/check-qom-interface.c
+++ b/tests/check-qom-interface.c
@@ -76,6 +76,7 @@ static void test_interface_impl(const char *type)
 
     g_assert(iobj);
     g_assert(ioc->test == PATTERN);
+    object_unref(obj);
 }
 
 static void interface_direct_test(void)
-- 
2.9.0

[Qemu-devel] [PATCH for-2.7 v4 10/36] tests: fix check-qom-proplist leaks

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Found thanks to ASAN.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 tests/check-qom-proplist.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/tests/check-qom-proplist.c b/tests/check-qom-proplist.c
index 42defe7..a16cefc 100644
--- a/tests/check-qom-proplist.c
+++ b/tests/check-qom-proplist.c
@@ -230,6 +230,13 @@ struct DummyBackendClass {
 };
 
 
+static void dummy_dev_finalize(Object *obj)
+{
+    DummyDev *dev = DUMMY_DEV(obj);
+
+    object_unref(OBJECT(dev->bus));
+}
+
 static void dummy_dev_init(Object *obj)
 {
     DummyDev *dev = DUMMY_DEV(obj);
@@ -257,6 +264,13 @@ static void dummy_dev_class_init(ObjectClass *klass, void *opaque)
 }
 
 
+static void dummy_bus_finalize(Object *obj)
+{
+    DummyBus *bus = DUMMY_BUS(obj);
+
+    object_unref(OBJECT(bus->backend));
+}
+
 static void dummy_bus_init(Object *obj)
 {
 }
@@ -283,6 +297,7 @@ static const TypeInfo dummy_dev_info = {
     .parent        = TYPE_OBJECT,
     .instance_size = sizeof(DummyDev),
     .instance_init = dummy_dev_init,
+    .instance_finalize = dummy_dev_finalize,
     .class_size = sizeof(DummyDevClass),
     .class_init = dummy_dev_class_init,
 };
@@ -292,6 +307,7 @@ static const TypeInfo dummy_bus_info = {
     .parent        = TYPE_OBJECT,
     .instance_size = sizeof(DummyBus),
     .instance_init = dummy_bus_init,
+    .instance_finalize = dummy_bus_finalize,
     .class_size = sizeof(DummyBusClass),
     .class_init = dummy_bus_class_init,
 };
-- 
2.9.0

[Qemu-devel] [PATCH for-2.7 v4 11/36] tests: fix small leak in test-io-channel-command

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

srcfifo && dstfifo must be freed in error case, however unlink() may
delete a file from a different context. Instead, use mkdtemp()/rmdir()
for the temporary files.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 tests/test-io-channel-command.c | 20 +++++++++++++-------
 tests/test-qga.c                |  3 ++-
 2 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/tests/test-io-channel-command.c b/tests/test-io-channel-command.c
index 1d1f461..f99118e 100644
--- a/tests/test-io-channel-command.c
+++ b/tests/test-io-channel-command.c
@@ -18,6 +18,7 @@
  *
  */
 
+#include <glib/gstdio.h>
 #include "qemu/osdep.h"
 #include "io/channel-command.h"
 #include "io-channel-helpers.h"
@@ -26,11 +27,14 @@
 #ifndef WIN32
 static void test_io_channel_command_fifo(bool async)
 {
-#define TEST_FIFO "tests/test-io-channel-command.fifo"
     QIOChannel *src, *dst;
     QIOChannelTest *test;
-    char *srcfifo = g_strdup_printf("PIPE:%s,wronly", TEST_FIFO);
-    char *dstfifo = g_strdup_printf("PIPE:%s,rdonly", TEST_FIFO);
+    char *tmpdir = g_strdup("/tmp/test-io-channel.XXXXXX");
+    g_assert_nonnull(mkdtemp(tmpdir));
+
+    char *fifo = g_strdup_printf("%s/command.fifo", tmpdir);
+    char *srcfifo = g_strdup_printf("PIPE:%s,wronly", fifo);
+    char *dstfifo = g_strdup_printf("PIPE:%s,rdonly", fifo);
     const char *srcargv[] = {
         "/bin/socat", "-", srcfifo, NULL,
     };
@@ -38,11 +42,10 @@ static void test_io_channel_command_fifo(bool async)
         "/bin/socat", dstfifo, "-", NULL,
     };
 
-    unlink(TEST_FIFO);
     if (access("/bin/socat", X_OK) < 0) {
-        return; /* Pretend success if socat is not present */
+        goto end; /* Pretend success if socat is not present */
     }
-    if (mkfifo(TEST_FIFO, 0600) < 0) {
+    if (mkfifo(fifo, 0600) < 0) {
         abort();
     }
     src = QIO_CHANNEL(qio_channel_command_new_spawn(srcargv,
@@ -59,9 +62,12 @@ static void test_io_channel_command_fifo(bool async)
     object_unref(OBJECT(src));
     object_unref(OBJECT(dst));
 
+end:
+    g_free(fifo);
     g_free(srcfifo);
     g_free(dstfifo);
-    unlink(TEST_FIFO);
+    g_rmdir(tmpdir);
+    g_free(tmpdir);
 }
 
 
diff --git a/tests/test-qga.c b/tests/test-qga.c
index 21f44f8..0d1acef 100644
--- a/tests/test-qga.c
+++ b/tests/test-qga.c
@@ -55,7 +55,8 @@ fixture_setup(TestFixture *fixture, gconstpointer data)
     fixture->loop = g_main_loop_new(NULL, FALSE);
 
     fixture->test_dir = g_strdup("/tmp/qgatest.XXXXXX");
-    g_assert_nonnull(mkdtemp(fixture->test_dir));
+    path = mkdtemp(fixture->test_dir);
+    g_assert_nonnull(path);
 
     path = g_build_filename(fixture->test_dir, "sock", NULL);
     cwd = g_get_current_dir();
-- 
2.9.0

[Qemu-devel] [PATCH for-2.7 v4 12/36] tests: fix leak in test-string-input-visitor

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Free the list returned by visit_type_intList().

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 tests/test-string-input-visitor.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/test-string-input-visitor.c b/tests/test-string-input-visitor.c
index d837ebe..a679fbc 100644
--- a/tests/test-string-input-visitor.c
+++ b/tests/test-string-input-visitor.c
@@ -228,6 +228,7 @@ static void test_visitor_in_fuzz(TestInputVisitorData *data,
 
         v = visitor_input_test_init(data, buf);
         visit_type_intList(v, NULL, &ilres, NULL);
+        qapi_free_intList(ilres);
         visitor_input_teardown(data, NULL);
 
         v = visitor_input_test_init(data, buf);
-- 
2.9.0

[Qemu-devel] [PATCH for-2.7 v4 13/36] portio: keep references on portio

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

The isa_register_portio_list() function allocates ioports
data/state. Let's keep the reference to this data on some owner.  This
isn't enough to fix leaks, but at least, ASAN stops complaining of
direct leaks. Further cleanup would require calling
portio_list_del/destroy().

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/audio/gus.c            |  9 ++++++---
 hw/audio/sb16.c           |  4 +++-
 hw/block/fdc.c            |  4 +++-
 hw/char/parallel.c        |  3 ++-
 hw/display/vga-isa.c      |  8 ++++++--
 hw/dma/i8257.c            |  6 ++++--
 hw/ide/core.c             |  6 ++++--
 hw/isa/isa-bus.c          | 14 +++++---------
 include/hw/ide/internal.h |  2 ++
 include/hw/isa/i8257.h    |  2 ++
 include/hw/isa/isa.h      |  5 ++++-
 11 files changed, 41 insertions(+), 22 deletions(-)

diff --git a/hw/audio/gus.c b/hw/audio/gus.c
index 6c02646..3d08a65 100644
--- a/hw/audio/gus.c
+++ b/hw/audio/gus.c
@@ -60,6 +60,8 @@ typedef struct GUSState {
     int64_t last_ticks;
     qemu_irq pic;
     IsaDma *isa_dma;
+    PortioList portio_list1;
+    PortioList portio_list2;
 } GUSState;
 
 static uint32_t gus_readb(void *opaque, uint32_t nport)
@@ -265,9 +267,10 @@ static void gus_realizefn (DeviceState *dev, Error **errp)
     s->samples = AUD_get_buffer_size_out (s->voice) >> s->shift;
     s->mixbuf = g_malloc0 (s->samples << s->shift);
 
-    isa_register_portio_list (d, s->port, gus_portio_list1, s, "gus");
-    isa_register_portio_list (d, (s->port + 0x100) & 0xf00,
-                              gus_portio_list2, s, "gus");
+    isa_register_portio_list(d, &s->portio_list1, s->port,
+                             gus_portio_list1, s, "gus");
+    isa_register_portio_list(d, &s->portio_list2, (s->port + 0x100) & 0xf00,
+                             gus_portio_list2, s, "gus");
 
     s->isa_dma = isa_get_dma(isa_bus_from_device(d), s->emu.gusdma);
     k = ISADMA_GET_CLASS(s->isa_dma);
diff --git a/hw/audio/sb16.c b/hw/audio/sb16.c
index 3a4a57a..6b4427f 100644
--- a/hw/audio/sb16.c
+++ b/hw/audio/sb16.c
@@ -106,6 +106,7 @@ typedef struct SB16State {
     /* mixer state */
     int mixer_nreg;
     uint8_t mixer_regs[256];
+    PortioList portio_list;
 } SB16State;
 
 static void SB_audio_callback (void *opaque, int free);
@@ -1378,7 +1379,8 @@ static void sb16_realizefn (DeviceState *dev, Error **errp)
         dolog ("warning: Could not create auxiliary timer\n");
     }
 
-    isa_register_portio_list (isadev, s->port, sb16_ioport_list, s, "sb16");
+    isa_register_portio_list(isadev, &s->portio_list, s->port,
+                             sb16_ioport_list, s, "sb16");
 
     s->isa_hdma = isa_get_dma(isa_bus_from_device(isadev), s->hdma);
     k = ISADMA_GET_CLASS(s->isa_hdma);
diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index f73af7d..b79873a 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -692,6 +692,7 @@ struct FDCtrl {
     /* Timers state */
     uint8_t timer0;
     uint8_t timer1;
+    PortioList portio_list;
 };
 
 static FloppyDriveType get_fallback_drive_type(FDrive *drv)
@@ -2495,7 +2496,8 @@ static void isabus_fdc_realize(DeviceState *dev, Error **errp)
     FDCtrl *fdctrl = &isa->state;
     Error *err = NULL;
 
-    isa_register_portio_list(isadev, isa->iobase, fdc_portio_list, fdctrl,
+    isa_register_portio_list(isadev, &fdctrl->portio_list,
+                             isa->iobase, fdc_portio_list, fdctrl,
                              "fdc");
 
     isa_init_irq(isadev, &fdctrl->irq, isa->irq);
diff --git a/hw/char/parallel.c b/hw/char/parallel.c
index 11c78fe..fa08566 100644
--- a/hw/char/parallel.c
+++ b/hw/char/parallel.c
@@ -80,6 +80,7 @@ typedef struct ParallelState {
     uint32_t last_read_offset; /* For debugging */
     /* Memory-mapped interface */
     int it_shift;
+    PortioList portio_list;
 } ParallelState;
 
 #define TYPE_ISA_PARALLEL "isa-parallel"
@@ -532,7 +533,7 @@ static void parallel_isa_realizefn(DeviceState *dev, Error **errp)
         s->status = dummy;
     }
 
-    isa_register_portio_list(isadev, base,
+    isa_register_portio_list(isadev, &s->portio_list, base,
                              (s->hw_driver
                               ? &isa_parallel_portio_hw_list[0]
                               : &isa_parallel_portio_sw_list[0]),
diff --git a/hw/display/vga-isa.c b/hw/display/vga-isa.c
index f5aff1c..1af9556 100644
--- a/hw/display/vga-isa.c
+++ b/hw/display/vga-isa.c
@@ -39,6 +39,8 @@ typedef struct ISAVGAState {
     ISADevice parent_obj;
 
     struct VGACommonState state;
+    PortioList portio_vga;
+    PortioList portio_vbe;
 } ISAVGAState;
 
 static void vga_isa_reset(DeviceState *dev)
@@ -60,9 +62,11 @@ static void vga_isa_realizefn(DeviceState *dev, Error **errp)
     vga_common_init(s, OBJECT(dev), true);
     s->legacy_address_space = isa_address_space(isadev);
     vga_io_memory = vga_init_io(s, OBJECT(dev), &vga_ports, &vbe_ports);
-    isa_register_portio_list(isadev, 0x3b0, vga_ports, s, "vga");
+    isa_register_portio_list(isadev, &d->portio_vga,
+                             0x3b0, vga_ports, s, "vga");
     if (vbe_ports) {
-        isa_register_portio_list(isadev, 0x1ce, vbe_ports, s, "vbe");
+        isa_register_portio_list(isadev, &d->portio_vbe,
+                                 0x1ce, vbe_ports, s, "vbe");
     }
     memory_region_add_subregion_overlap(isa_address_space(isadev),
                                         0x000a0000,
diff --git a/hw/dma/i8257.c b/hw/dma/i8257.c
index f345c54..bffbdea 100644
--- a/hw/dma/i8257.c
+++ b/hw/dma/i8257.c
@@ -553,10 +553,12 @@ static void i8257_realize(DeviceState *dev, Error **errp)
     memory_region_add_subregion(isa_address_space_io(isa),
                                 d->base, &d->channel_io);
 
-    isa_register_portio_list(isa, d->page_base, page_portio_list, d,
+    isa_register_portio_list(isa, &d->portio_page,
+                             d->page_base, page_portio_list, d,
                              "dma-page");
     if (d->pageh_base >= 0) {
-        isa_register_portio_list(isa, d->pageh_base, pageh_portio_list, d,
+        isa_register_portio_list(isa, &d->portio_pageh,
+                                 d->pageh_base, pageh_portio_list, d,
                                  "dma-pageh");
     }
 
diff --git a/hw/ide/core.c b/hw/ide/core.c
index d117b7c..f9c8162 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -2618,10 +2618,12 @@ void ide_init_ioport(IDEBus *bus, ISADevice *dev, int iobase, int iobase2)
 {
     /* ??? Assume only ISA and PCI configurations, and that the PCI-ISA
        bridge has been setup properly to always register with ISA.  */
-    isa_register_portio_list(dev, iobase, ide_portio_list, bus, "ide");
+    isa_register_portio_list(dev, &bus->portio_list,
+                             iobase, ide_portio_list, bus, "ide");
 
     if (iobase2) {
-        isa_register_portio_list(dev, iobase2, ide_portio2_list, bus, "ide");
+        isa_register_portio_list(dev, &bus->portio2_list,
+                                 iobase2, ide_portio2_list, bus, "ide");
     }
 }
 
diff --git a/hw/isa/isa-bus.c b/hw/isa/isa-bus.c
index ce74db2..9d07b11 100644
--- a/hw/isa/isa-bus.c
+++ b/hw/isa/isa-bus.c
@@ -131,24 +131,20 @@ void isa_register_ioport(ISADevice *dev, MemoryRegion *io, uint16_t start)
     isa_init_ioport(dev, start);
 }
 
-void isa_register_portio_list(ISADevice *dev, uint16_t start,
+void isa_register_portio_list(ISADevice *dev,
+                              PortioList *piolist, uint16_t start,
                               const MemoryRegionPortio *pio_start,
                               void *opaque, const char *name)
 {
-    PortioList piolist;
+    assert(piolist && !piolist->owner);
 
     /* START is how we should treat DEV, regardless of the actual
        contents of the portio array.  This is how the old code
        actually handled e.g. the FDC device.  */
     isa_init_ioport(dev, start);
 
-    /* FIXME: the device should store created PortioList in its state.  Note
-       that DEV can be NULL here and that single device can register several
-       portio lists.  Current implementation is leaking memory allocated
-       in portio_list_init.  The leak is not critical because it happens only
-       at initialization time.  */
-    portio_list_init(&piolist, OBJECT(dev), pio_start, opaque, name);
-    portio_list_add(&piolist, isabus->address_space_io, start);
+    portio_list_init(piolist, OBJECT(dev), pio_start, opaque, name);
+    portio_list_add(piolist, isabus->address_space_io, start);
 }
 
 static void isa_device_init(Object *obj)
diff --git a/include/hw/ide/internal.h b/include/hw/ide/internal.h
index 7824bc3..a6dd2c3 100644
--- a/include/hw/ide/internal.h
+++ b/include/hw/ide/internal.h
@@ -480,6 +480,8 @@ struct IDEBus {
     uint8_t retry_unit;
     int64_t retry_sector_num;
     uint32_t retry_nsector;
+    PortioList portio_list;
+    PortioList portio2_list;
 };
 
 #define TYPE_IDE_DEVICE "ide-device"
diff --git a/include/hw/isa/i8257.h b/include/hw/isa/i8257.h
index aa211c0..88a2766 100644
--- a/include/hw/isa/i8257.h
+++ b/include/hw/isa/i8257.h
@@ -36,6 +36,8 @@ typedef struct I8257State {
     QEMUBH *dma_bh;
     bool dma_bh_scheduled;
     int running;
+    PortioList portio_page;
+    PortioList portio_pageh;
 } I8257State;
 
 #endif
diff --git a/include/hw/isa/isa.h b/include/hw/isa/isa.h
index 7693ac5..c2fdd70 100644
--- a/include/hw/isa/isa.h
+++ b/include/hw/isa/isa.h
@@ -134,12 +134,15 @@ void isa_register_ioport(ISADevice *dev, MemoryRegion *io, uint16_t start);
  * device and use the legacy portio routines.
  *
  * @dev: the ISADevice against which these are registered; may be NULL.
+ * @piolist: the PortioList associated with the io ports
  * @start: the base I/O port against which the portio->offset is applied.
  * @portio: the ports, sorted by offset.
  * @opaque: passed into the portio callbacks.
  * @name: passed into memory_region_init_io.
  */
-void isa_register_portio_list(ISADevice *dev, uint16_t start,
+void isa_register_portio_list(ISADevice *dev,
+                              PortioList *piolist,
+                              uint16_t start,
                               const MemoryRegionPortio *portio,
                               void *opaque, const char *name);
 
-- 
2.9.0

[Qemu-devel] [PATCH for-2.7 v4 14/36] numa: do not leak NumaOptions

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

In all cases, call qapi_free_NumaOptions(), by using a common ending
block.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 numa.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/numa.c b/numa.c
index 7286171..6289f46 100644
--- a/numa.c
+++ b/numa.c
@@ -223,14 +223,14 @@ static int parse_numa(void *opaque, QemuOpts *opts, Error **errp)
     }
 
     if (err) {
-        goto error;
+        goto end;
     }
 
     switch (object->type) {
     case NUMA_OPTIONS_KIND_NODE:
         numa_node_parse(object->u.node.data, opts, &err);
         if (err) {
-            goto error;
+            goto end;
         }
         nb_numa_nodes++;
         break;
@@ -238,13 +238,14 @@ static int parse_numa(void *opaque, QemuOpts *opts, Error **errp)
         abort();
     }
 
-    return 0;
-
-error:
-    error_report_err(err);
+end:
     qapi_free_NumaOptions(object);
+    if (err) {
+        error_report_err(err);
+        return -1;
+    }
 
-    return -1;
+    return 0;
 }
 
 static char *enumerate_cpus(unsigned long *cpus, int max_cpus)
-- 
2.9.0

[Qemu-devel] [PATCH for-2.7 v4 15/36] pc: simplify passing qemu_irq

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

qemu_irq is already a pointer, no need to have an extra pointer level.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 hw/i386/pc.c         | 8 ++++----
 hw/input/pckbd.c     | 4 ++--
 include/hw/i386/pc.h | 2 +-
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 47593b7..6b138d6 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -530,9 +530,9 @@ static uint64_t port92_read(void *opaque, hwaddr addr,
     return ret;
 }
 
-static void port92_init(ISADevice *dev, qemu_irq *a20_out)
+static void port92_init(ISADevice *dev, qemu_irq a20_out)
 {
-    qdev_connect_gpio_out_named(DEVICE(dev), PORT92_A20_LINE, 0, *a20_out);
+    qdev_connect_gpio_out_named(DEVICE(dev), PORT92_A20_LINE, 0, a20_out);
 }
 
 static const VMStateDescription vmstate_port92_isa = {
@@ -1594,7 +1594,7 @@ void pc_basic_device_init(ISABus *isa_bus, qemu_irq *gsi,
 
     a20_line = qemu_allocate_irqs(handle_a20_line_change, first_cpu, 2);
     i8042 = isa_create_simple(isa_bus, "i8042");
-    i8042_setup_a20_line(i8042, &a20_line[0]);
+    i8042_setup_a20_line(i8042, a20_line[0]);
     if (!no_vmport) {
         vmport_init(isa_bus);
         vmmouse = isa_try_create(isa_bus, "vmmouse");
@@ -1607,7 +1607,7 @@ void pc_basic_device_init(ISABus *isa_bus, qemu_irq *gsi,
         qdev_init_nofail(dev);
     }
     port92 = isa_create_simple(isa_bus, "port92");
-    port92_init(port92, &a20_line[1]);
+    port92_init(port92, a20_line[1]);
 
     DMA_init(isa_bus, 0);
 
diff --git a/hw/input/pckbd.c b/hw/input/pckbd.c
index dc57e2c..d414288 100644
--- a/hw/input/pckbd.c
+++ b/hw/input/pckbd.c
@@ -499,9 +499,9 @@ void i8042_isa_mouse_fake_event(void *opaque)
     ps2_mouse_fake_event(s->mouse);
 }
 
-void i8042_setup_a20_line(ISADevice *dev, qemu_irq *a20_out)
+void i8042_setup_a20_line(ISADevice *dev, qemu_irq a20_out)
 {
-    qdev_connect_gpio_out_named(DEVICE(dev), I8042_A20_LINE, 0, *a20_out);
+    qdev_connect_gpio_out_named(DEVICE(dev), I8042_A20_LINE, 0, a20_out);
 }
 
 static const VMStateDescription vmstate_kbd_isa = {
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 74c175c..330c1f2 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -220,7 +220,7 @@ void i8042_mm_init(qemu_irq kbd_irq, qemu_irq mouse_irq,
                    MemoryRegion *region, ram_addr_t size,
                    hwaddr mask);
 void i8042_isa_mouse_fake_event(void *opaque);
-void i8042_setup_a20_line(ISADevice *dev, qemu_irq *a20_out);
+void i8042_setup_a20_line(ISADevice *dev, qemu_irq a20_out);
 
 /* pc.c */
 extern int fd_bootchk;
-- 
2.9.0

[Qemu-devel] [PATCH for-2.7 v4 16/36] pc: don't leak a20_line

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

The irqs array is no longer being used

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 hw/i386/pc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 6b138d6..fd4a050 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1608,6 +1608,7 @@ void pc_basic_device_init(ISABus *isa_bus, qemu_irq *gsi,
     }
     port92 = isa_create_simple(isa_bus, "port92");
     port92_init(port92, a20_line[1]);
+    g_free(a20_line);
 
     DMA_init(isa_bus, 0);
 
-- 
2.9.0

[Qemu-devel] [PATCH for-2.7 v4 17/36] machine: use class base init generated name

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

machine_class_base_init() member name is allocated by
machine_class_base_init(), but not freed by
machine_class_finalize().  Simply freeing there doesn't work,
because DEFINE_PC_MACHINE() overwrites it with a literal string.

Fix DEFINE_PC_MACHINE() not to overwrite it, and add the missing
free to machine_class_finalize().

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
---
 hw/core/machine.c    | 1 +
 include/hw/boards.h  | 2 +-
 include/hw/i386/pc.h | 1 -
 3 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/core/machine.c b/hw/core/machine.c
index e5a456f..00fbe3e 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -561,6 +561,7 @@ static void machine_class_finalize(ObjectClass *klass, void *data)
     if (mc->compat_props) {
         g_array_free(mc->compat_props, true);
     }
+    g_free(mc->name);
 }
 
 void machine_register_compat_props(MachineState *machine)
diff --git a/include/hw/boards.h b/include/hw/boards.h
index 3e69eca..e46a744 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -93,7 +93,7 @@ struct MachineClass {
     /*< public >*/
 
     const char *family; /* NULL iff @name identifies a standalone machtype */
-    const char *name;
+    char *name;
     const char *alias;
     const char *desc;
 
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 330c1f2..422fac7 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -903,7 +903,6 @@ bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
     { \
         MachineClass *mc = MACHINE_CLASS(oc); \
         optsfn(mc); \
-        mc->name = namestr; \
         mc->init = initfn; \
     } \
     static const TypeInfo pc_machine_type_##suffix = { \
-- 
2.9.0

[Qemu-devel] [PATCH for-2.7 v4 18/36] acpi-build: fix array leak

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

The free_ranges array is used as a temporary pointer array, the segment
should still be freed, however, it shouldn't free the elements themself.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Tested-by: Marcel Apfelbaum <marcel@redhat.com>
Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>
---
 hw/i386/acpi-build.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index a26a4bb..433feba 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -789,7 +789,7 @@ static gint crs_range_compare(gconstpointer a, gconstpointer b)
 static void crs_replace_with_free_ranges(GPtrArray *ranges,
                                          uint64_t start, uint64_t end)
 {
-    GPtrArray *free_ranges = g_ptr_array_new_with_free_func(crs_range_free);
+    GPtrArray *free_ranges = g_ptr_array_new();
     uint64_t free_base = start;
     int i;
 
@@ -813,7 +813,7 @@ static void crs_replace_with_free_ranges(GPtrArray *ranges,
         g_ptr_array_add(ranges, g_ptr_array_index(free_ranges, i));
     }
 
-    g_ptr_array_free(free_ranges, false);
+    g_ptr_array_free(free_ranges, true);
 }
 
 /*
-- 
2.9.0

[Qemu-devel] [PATCH for-2.7 v4 19/36] char: free the tcp connection data when closing

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Make sure the connection data got freed when closing the chardev, to
avoid leaks. Introduce tcp_chr_free_connection() to clean all connection
related data, and move some tcp_chr_close() clean-ups there.

(while at it, set write_msgfds_num to 0 when clearing array in
tcp_set_msgfds())

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 qemu-char.c | 50 +++++++++++++++++++++++++++++++-------------------
 1 file changed, 31 insertions(+), 19 deletions(-)

diff --git a/qemu-char.c b/qemu-char.c
index a50b8fb..f20d385 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2763,6 +2763,7 @@ static int tcp_set_msgfds(CharDriverState *chr, int *fds, int num)
     /* clear old pending fd array */
     g_free(s->write_msgfds);
     s->write_msgfds = NULL;
+    s->write_msgfds_num = 0;
 
     if (!s->connected ||
         !qio_channel_has_feature(s->ioc,
@@ -2843,19 +2844,24 @@ static GSource *tcp_chr_add_watch(CharDriverState *chr, GIOCondition cond)
     return qio_channel_create_watch(s->ioc, cond);
 }
 
-static void tcp_chr_disconnect(CharDriverState *chr)
+static void tcp_chr_free_connection(CharDriverState *chr)
 {
     TCPCharDriver *s = chr->opaque;
+    int i;
 
     if (!s->connected) {
         return;
     }
 
-    s->connected = 0;
-    if (s->listen_ioc) {
-        s->listen_tag = qio_channel_add_watch(
-            QIO_CHANNEL(s->listen_ioc), G_IO_IN, tcp_chr_accept, chr, NULL);
+    if (s->read_msgfds_num) {
+        for (i = 0; i < s->read_msgfds_num; i++) {
+            close(s->read_msgfds[i]);
+        }
+        g_free(s->read_msgfds);
+        s->read_msgfds = NULL;
+        s->read_msgfds_num = 0;
     }
+
     tcp_set_msgfds(chr, NULL, 0);
     remove_fd_in_watch(chr);
     object_unref(OBJECT(s->sioc));
@@ -2863,6 +2869,24 @@ static void tcp_chr_disconnect(CharDriverState *chr)
     object_unref(OBJECT(s->ioc));
     s->ioc = NULL;
     g_free(chr->filename);
+    chr->filename = NULL;
+    s->connected = 0;
+}
+
+static void tcp_chr_disconnect(CharDriverState *chr)
+{
+    TCPCharDriver *s = chr->opaque;
+
+    if (!s->connected) {
+        return;
+    }
+
+    tcp_chr_free_connection(chr);
+
+    if (s->listen_ioc) {
+        s->listen_tag = qio_channel_add_watch(
+            QIO_CHANNEL(s->listen_ioc), G_IO_IN, tcp_chr_accept, chr, NULL);
+    }
     chr->filename = SocketAddress_to_str("disconnected:", s->addr,
                                          s->is_listen, s->is_telnet);
     qemu_chr_be_event(chr, CHR_EVENT_CLOSED);
@@ -3177,17 +3201,14 @@ int qemu_chr_wait_connected(CharDriverState *chr, Error **errp)
 static void tcp_chr_close(CharDriverState *chr)
 {
     TCPCharDriver *s = chr->opaque;
-    int i;
+
+    tcp_chr_free_connection(chr);
 
     if (s->reconnect_timer) {
         g_source_remove(s->reconnect_timer);
         s->reconnect_timer = 0;
     }
     qapi_free_SocketAddress(s->addr);
-    remove_fd_in_watch(chr);
-    if (s->ioc) {
-        object_unref(OBJECT(s->ioc));
-    }
     if (s->listen_tag) {
         g_source_remove(s->listen_tag);
         s->listen_tag = 0;
@@ -3195,18 +3216,9 @@ static void tcp_chr_close(CharDriverState *chr)
     if (s->listen_ioc) {
         object_unref(OBJECT(s->listen_ioc));
     }
-    if (s->read_msgfds_num) {
-        for (i = 0; i < s->read_msgfds_num; i++) {
-            close(s->read_msgfds[i]);
-        }
-        g_free(s->read_msgfds);
-    }
     if (s->tls_creds) {
         object_unref(OBJECT(s->tls_creds));
     }
-    if (s->write_msgfds_num) {
-        g_free(s->write_msgfds);
-    }
     g_free(s);
     qemu_chr_be_event(chr, CHR_EVENT_CLOSED);
 }
-- 
2.9.0

[Qemu-devel] [PATCH for-2.7 v4 20/36] char: free MuxDriver when closing

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Similarly to other chr_close callbacks, free char type specific data.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 qemu-char.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/qemu-char.c b/qemu-char.c
index f20d385..539dd70 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -786,6 +786,13 @@ static GSource *mux_chr_add_watch(CharDriverState *s, GIOCondition cond)
     return d->drv->chr_add_watch(d->drv, cond);
 }
 
+static void mux_chr_close(struct CharDriverState *chr)
+{
+    MuxDriver *d = chr->opaque;
+
+    g_free(d);
+}
+
 static CharDriverState *qemu_chr_open_mux(const char *id,
                                           ChardevBackend *backend,
                                           ChardevReturn *ret, Error **errp)
@@ -810,6 +817,7 @@ static CharDriverState *qemu_chr_open_mux(const char *id,
     chr->opaque = d;
     d->drv = drv;
     d->focus = -1;
+    chr->chr_close = mux_chr_close;
     chr->chr_write = mux_chr_write;
     chr->chr_update_read_handler = mux_chr_update_read_handler;
     chr->chr_accept_input = mux_chr_accept_input;
-- 
2.9.0

[Qemu-devel] [PATCH for-2.7 v4 21/36] tests: fix qom-test leaks

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 tests/qom-test.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/tests/qom-test.c b/tests/qom-test.c
index 23493a2..d48f890 100644
--- a/tests/qom-test.c
+++ b/tests/qom-test.c
@@ -115,7 +115,7 @@ static void add_machine_test_cases(void)
     const QListEntry *p;
     QObject *qobj;
     QString *qstr;
-    const char *mname, *path;
+    const char *mname;
 
     qtest_start("-machine none");
     response = qmp("{ 'execute': 'query-machines' }");
@@ -132,8 +132,9 @@ static void add_machine_test_cases(void)
         g_assert(qstr);
         mname = qstring_get_str(qstr);
         if (!is_blacklisted(arch, mname)) {
-            path = g_strdup_printf("qom/%s", mname);
+            char *path = g_strdup_printf("qom/%s", mname);
             qtest_add_data_func(path, g_strdup(mname), test_machine);
+            g_free(path);
         }
     }
 
-- 
2.9.0

[Qemu-devel] [PATCH for-2.7 v4 22/36] pc: free i8259

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Simiarly to 2ba154cf4eb8636cdd3aa90f392ca9e77206ca39

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>
---
 hw/i386/pc_q35.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index c0b9961..c5e8367 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -213,6 +213,8 @@ static void pc_q35_init(MachineState *machine)
     for (i = 0; i < ISA_NUM_IRQS; i++) {
         gsi_state->i8259_irq[i] = i8259[i];
     }
+    g_free(i8259);
+
     if (pcmc->pci_enabled) {
         ioapic_init_gsi(gsi_state, "q35");
     }
-- 
2.9.0

[Qemu-devel] [PATCH for-2.7 v4 23/36] pc: keep gsi reference

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Further cleanup would need to call qemu_free_irq() at the appropriate
time, but for now this silences ASAN about direct leaks.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
---
 hw/i386/pc_piix.c    | 17 ++++++++---------
 hw/i386/pc_q35.c     | 13 ++++++-------
 include/hw/i386/pc.h |  1 +
 3 files changed, 15 insertions(+), 16 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index a07dc81..2af8888 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -74,7 +74,6 @@ static void pc_init1(MachineState *machine,
     ISABus *isa_bus;
     PCII440FXState *i440fx_state;
     int piix3_devfn = -1;
-    qemu_irq *gsi;
     qemu_irq *i8259;
     qemu_irq smi_irq;
     GSIState *gsi_state;
@@ -185,16 +184,16 @@ static void pc_init1(MachineState *machine,
     gsi_state = g_malloc0(sizeof(*gsi_state));
     if (kvm_ioapic_in_kernel()) {
         kvm_pc_setup_irq_routing(pcmc->pci_enabled);
-        gsi = qemu_allocate_irqs(kvm_pc_gsi_handler, gsi_state,
-                                 GSI_NUM_PINS);
+        pcms->gsi = qemu_allocate_irqs(kvm_pc_gsi_handler, gsi_state,
+                                       GSI_NUM_PINS);
     } else {
-        gsi = qemu_allocate_irqs(gsi_handler, gsi_state, GSI_NUM_PINS);
+        pcms->gsi = qemu_allocate_irqs(gsi_handler, gsi_state, GSI_NUM_PINS);
     }
 
     if (pcmc->pci_enabled) {
         pci_bus = i440fx_init(host_type,
                               pci_type,
-                              &i440fx_state, &piix3_devfn, &isa_bus, gsi,
+                              &i440fx_state, &piix3_devfn, &isa_bus, pcms->gsi,
                               system_memory, system_io, machine->ram_size,
                               pcms->below_4g_mem_size,
                               pcms->above_4g_mem_size,
@@ -207,7 +206,7 @@ static void pc_init1(MachineState *machine,
                               &error_abort);
         no_hpet = 1;
     }
-    isa_bus_irqs(isa_bus, gsi);
+    isa_bus_irqs(isa_bus, pcms->gsi);
 
     if (kvm_pic_in_kernel()) {
         i8259 = kvm_i8259_init(isa_bus);
@@ -225,7 +224,7 @@ static void pc_init1(MachineState *machine,
         ioapic_init_gsi(gsi_state, "i440fx");
     }
 
-    pc_register_ferr_irq(gsi[13]);
+    pc_register_ferr_irq(pcms->gsi[13]);
 
     pc_vga_init(isa_bus, pcmc->pci_enabled ? pci_bus : NULL);
 
@@ -235,7 +234,7 @@ static void pc_init1(MachineState *machine,
     }
 
     /* init basic PC hardware */
-    pc_basic_device_init(isa_bus, gsi, &rtc_state, true,
+    pc_basic_device_init(isa_bus, pcms->gsi, &rtc_state, true,
                          (pcms->vmport != ON_OFF_AUTO_ON), 0x4);
 
     pc_nic_init(isa_bus, pci_bus);
@@ -279,7 +278,7 @@ static void pc_init1(MachineState *machine,
         smi_irq = qemu_allocate_irq(pc_acpi_smi_interrupt, first_cpu, 0);
         /* TODO: Populate SPD eeprom data.  */
         smbus = piix4_pm_init(pci_bus, piix3_devfn + 3, 0xb100,
-                              gsi[9], smi_irq,
+                              pcms->gsi[9], smi_irq,
                               pc_machine_is_smm_enabled(pcms),
                               &piix4_pm);
         smbus_eeprom_init(smbus, 8, NULL, 0);
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index c5e8367..3cbcbb0 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -69,7 +69,6 @@ static void pc_q35_init(MachineState *machine)
     MemoryRegion *ram_memory;
     GSIState *gsi_state;
     ISABus *isa_bus;
-    qemu_irq *gsi;
     qemu_irq *i8259;
     int i;
     ICH9LPCState *ich9_lpc;
@@ -153,10 +152,10 @@ static void pc_q35_init(MachineState *machine)
     gsi_state = g_malloc0(sizeof(*gsi_state));
     if (kvm_ioapic_in_kernel()) {
         kvm_pc_setup_irq_routing(pcmc->pci_enabled);
-        gsi = qemu_allocate_irqs(kvm_pc_gsi_handler, gsi_state,
-                                 GSI_NUM_PINS);
+        pcms->gsi = qemu_allocate_irqs(kvm_pc_gsi_handler, gsi_state,
+                                       GSI_NUM_PINS);
     } else {
-        gsi = qemu_allocate_irqs(gsi_handler, gsi_state, GSI_NUM_PINS);
+        pcms->gsi = qemu_allocate_irqs(gsi_handler, gsi_state, GSI_NUM_PINS);
     }
 
     /* create pci host bus */
@@ -195,7 +194,7 @@ static void pc_q35_init(MachineState *machine)
     ich9_lpc = ICH9_LPC_DEVICE(lpc);
     lpc_dev = DEVICE(lpc);
     for (i = 0; i < GSI_NUM_PINS; i++) {
-        qdev_connect_gpio_out_named(lpc_dev, ICH9_GPIO_GSI, i, gsi[i]);
+        qdev_connect_gpio_out_named(lpc_dev, ICH9_GPIO_GSI, i, pcms->gsi[i]);
     }
     pci_bus_irqs(host_bus, ich9_lpc_set_irq, ich9_lpc_map_irq, ich9_lpc,
                  ICH9_LPC_NB_PIRQS);
@@ -219,7 +218,7 @@ static void pc_q35_init(MachineState *machine)
         ioapic_init_gsi(gsi_state, "q35");
     }
 
-    pc_register_ferr_irq(gsi[13]);
+    pc_register_ferr_irq(pcms->gsi[13]);
 
     assert(pcms->vmport != ON_OFF_AUTO__MAX);
     if (pcms->vmport == ON_OFF_AUTO_AUTO) {
@@ -227,7 +226,7 @@ static void pc_q35_init(MachineState *machine)
     }
 
     /* init basic PC hardware */
-    pc_basic_device_init(isa_bus, gsi, &rtc_state, !mc->no_floppy,
+    pc_basic_device_init(isa_bus, pcms->gsi, &rtc_state, !mc->no_floppy,
                          (pcms->vmport != ON_OFF_AUTO_ON), 0xff0104);
 
     /* connect pm stuff to lpc */
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 422fac7..c4316c3 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -53,6 +53,7 @@ struct PCMachineState {
     ISADevice *rtc;
     PCIBus *bus;
     FWCfgState *fw_cfg;
+    qemu_irq *gsi;
 
     /* Configuration options: */
     uint64_t max_ram_below_4g;
-- 
2.9.0

[Qemu-devel] [PATCH for-2.7 v4 24/36] ahci: free irqs array

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Each irq is referenced by the IDEBus in ide_init2(), thus we can free
the no longer used array.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Acked-by: John Snow <jsnow@redhat.com>
---
 hw/ide/ahci.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index bcb9ff9..6defeed 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -1478,6 +1478,7 @@ void ahci_realize(AHCIState *s, DeviceState *qdev, AddressSpace *as, int ports)
         ad->port.dma->ops = &ahci_dma_ops;
         ide_register_restart_cb(&ad->port);
     }
+    g_free(irqs);
 }
 
 void ahci_uninit(AHCIState *s)
-- 
2.9.0

[Qemu-devel] [PATCH for-2.7 v4 25/36] sd: free timer

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Free the timer allocated in instance_init.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Andrew Baumann <Andrew.Baumann@microsoft.com>
---
 hw/sd/sd.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 87c6dc1..8e88e83 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1876,6 +1876,14 @@ static void sd_instance_init(Object *obj)
     sd->ocr_power_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, sd_ocr_powerup, sd);
 }
 
+static void sd_instance_finalize(Object *obj)
+{
+    SDState *sd = SD_CARD(obj);
+
+    timer_del(sd->ocr_power_timer);
+    timer_free(sd->ocr_power_timer);
+}
+
 static void sd_realize(DeviceState *dev, Error **errp)
 {
     SDState *sd = SD_CARD(dev);
@@ -1927,6 +1935,7 @@ static const TypeInfo sd_info = {
     .class_size = sizeof(SDCardClass),
     .class_init = sd_class_init,
     .instance_init = sd_instance_init,
+    .instance_finalize = sd_instance_finalize,
 };
 
 static void sd_register_types(void)
-- 
2.9.0

[Qemu-devel] [PATCH for-2.7 v4 26/36] qjson: free str

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Release the qstring allocated in qjson_new().

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 migration/qjson.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/migration/qjson.c b/migration/qjson.c
index 5cae55a..f345904 100644
--- a/migration/qjson.c
+++ b/migration/qjson.c
@@ -109,5 +109,6 @@ void qjson_finish(QJSON *json)
 
 void qjson_destroy(QJSON *json)
 {
+    QDECREF(json->str);
     g_free(json);
 }
-- 
2.9.0

[Qemu-devel] [PATCH for-2.7 v4 27/36] virtio-input: free config list

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Clear the list when finalizing. The list is created during realize with
virtio_input_idstr_config() and later by further calls to
virtio_input_init_config() and virtio_input_add_config().

This leak can be reproduced with device-introspect-test -p
/x86_64/device/introspect/concrete.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/input/virtio-input.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/hw/input/virtio-input.c b/hw/input/virtio-input.c
index a87fd68..ccdf730 100644
--- a/hw/input/virtio-input.c
+++ b/hw/input/virtio-input.c
@@ -270,6 +270,16 @@ static void virtio_input_device_realize(DeviceState *dev, Error **errp)
     vinput->sts = virtio_add_queue(vdev, 64, virtio_input_handle_sts);
 }
 
+static void virtio_input_finalize(Object *obj)
+{
+    VirtIOInput *vinput = VIRTIO_INPUT(obj);
+    VirtIOInputConfig *cfg, *next;
+
+    QTAILQ_FOREACH_SAFE(cfg, &vinput->cfg_list, node, next) {
+        QTAILQ_REMOVE(&vinput->cfg_list, cfg, node);
+        g_free(cfg);
+    }
+}
 static void virtio_input_device_unrealize(DeviceState *dev, Error **errp)
 {
     VirtIOInputClass *vic = VIRTIO_INPUT_GET_CLASS(dev);
@@ -318,6 +328,7 @@ static const TypeInfo virtio_input_info = {
     .class_size    = sizeof(VirtIOInputClass),
     .class_init    = virtio_input_class_init,
     .abstract      = true,
+    .instance_finalize = virtio_input_finalize,
 };
 
 /* ----------------------------------------------------------------- */
-- 
2.9.0

[Qemu-devel] [PATCH for-2.7 v4 28/36] ipmi: free extern timer

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Free the timer allocated during instance init.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Corey Minyard <cminyard@mvista.com>
---
 hw/ipmi/ipmi_bmc_extern.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/hw/ipmi/ipmi_bmc_extern.c b/hw/ipmi/ipmi_bmc_extern.c
index 157879e..5b73983 100644
--- a/hw/ipmi/ipmi_bmc_extern.c
+++ b/hw/ipmi/ipmi_bmc_extern.c
@@ -487,6 +487,14 @@ static void ipmi_bmc_extern_init(Object *obj)
     vmstate_register(NULL, 0, &vmstate_ipmi_bmc_extern, ibe);
 }
 
+static void ipmi_bmc_extern_finalize(Object *obj)
+{
+    IPMIBmcExtern *ibe = IPMI_BMC_EXTERN(obj);
+
+    timer_del(ibe->extern_timer);
+    timer_free(ibe->extern_timer);
+}
+
 static Property ipmi_bmc_extern_properties[] = {
     DEFINE_PROP_CHR("chardev", IPMIBmcExtern, chr),
     DEFINE_PROP_END_OF_LIST(),
@@ -508,6 +516,7 @@ static const TypeInfo ipmi_bmc_extern_type = {
     .parent        = TYPE_IPMI_BMC,
     .instance_size = sizeof(IPMIBmcExtern),
     .instance_init = ipmi_bmc_extern_init,
+    .instance_finalize = ipmi_bmc_extern_finalize,
     .class_init    = ipmi_bmc_extern_class_init,
  };
 
-- 
2.9.0

[Qemu-devel] [PATCH for-2.7 v4 29/36] usb: free USBDevice.strings

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

The list is created during instance init and further populated with
usb_desc_set_string(). Clear it when unrealizing the device.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb/bus.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/hw/usb/bus.c b/hw/usb/bus.c
index c28ccb8..25913ad 100644
--- a/hw/usb/bus.c
+++ b/hw/usb/bus.c
@@ -279,6 +279,13 @@ static void usb_qdev_realize(DeviceState *qdev, Error **errp)
 static void usb_qdev_unrealize(DeviceState *qdev, Error **errp)
 {
     USBDevice *dev = USB_DEVICE(qdev);
+    USBDescString *s, *next;
+
+    QLIST_FOREACH_SAFE(s, &dev->strings, next, next) {
+        QLIST_REMOVE(s, next);
+        g_free(s->str);
+        g_free(s);
+    }
 
     if (dev->attached) {
         usb_device_detach(dev);
-- 
2.9.0

[Qemu-devel] [PATCH for-2.7 v4 30/36] usb: free leaking path

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

qdev_get_dev_path() returns an allocated string, free it when no longer
needed.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb/desc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/usb/desc.c b/hw/usb/desc.c
index adb026e..5e0e1d1 100644
--- a/hw/usb/desc.c
+++ b/hw/usb/desc.c
@@ -574,6 +574,7 @@ void usb_desc_create_serial(USBDevice *dev)
     }
     dst += snprintf(serial+dst, sizeof(serial)-dst, "-%s", dev->port->path);
     usb_desc_set_string(dev, index, serial);
+    g_free(path);
 }
 
 const char *usb_desc_get_string(USBDevice *dev, uint8_t index)
-- 
2.9.0

[Qemu-devel] [PATCH for-2.7 v4 31/36] bus: simplify name handling

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Simplify a bit the code by using g_strdup_printf() and store it in a
non-const value so casting is no longer needed, and ownership is
clearer.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 hw/core/bus.c          | 21 ++++++---------------
 include/hw/qdev-core.h |  2 +-
 2 files changed, 7 insertions(+), 16 deletions(-)

diff --git a/hw/core/bus.c b/hw/core/bus.c
index 3e3f8ac..cf383fc 100644
--- a/hw/core/bus.c
+++ b/hw/core/bus.c
@@ -78,8 +78,7 @@ static void qbus_realize(BusState *bus, DeviceState *parent, const char *name)
 {
     const char *typename = object_get_typename(OBJECT(bus));
     BusClass *bc;
-    char *buf;
-    int i, len, bus_id;
+    int i, bus_id;
 
     bus->parent = parent;
 
@@ -88,23 +87,15 @@ static void qbus_realize(BusState *bus, DeviceState *parent, const char *name)
     } else if (bus->parent && bus->parent->id) {
         /* parent device has id -> use it plus parent-bus-id for bus name */
         bus_id = bus->parent->num_child_bus;
-
-        len = strlen(bus->parent->id) + 16;
-        buf = g_malloc(len);
-        snprintf(buf, len, "%s.%d", bus->parent->id, bus_id);
-        bus->name = buf;
+        bus->name = g_strdup_printf("%s.%d", bus->parent->id, bus_id);
     } else {
         /* no id -> use lowercase bus type plus global bus-id for bus name */
         bc = BUS_GET_CLASS(bus);
         bus_id = bc->automatic_ids++;
-
-        len = strlen(typename) + 16;
-        buf = g_malloc(len);
-        len = snprintf(buf, len, "%s.%d", typename, bus_id);
-        for (i = 0; i < len; i++) {
-            buf[i] = qemu_tolower(buf[i]);
+        bus->name = g_strdup_printf("%s.%d", typename, bus_id);
+        for (i = 0; bus->name[i]; i++) {
+            bus->name[i] = qemu_tolower(bus->name[i]);
         }
-        bus->name = buf;
     }
 
     if (bus->parent) {
@@ -229,7 +220,7 @@ static void qbus_finalize(Object *obj)
 {
     BusState *bus = BUS(obj);
 
-    g_free((char *)bus->name);
+    g_free(bus->name);
 }
 
 static const TypeInfo bus_info = {
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 4b4b33b..2c97347 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -224,7 +224,7 @@ typedef struct BusChild {
 struct BusState {
     Object obj;
     DeviceState *parent;
-    const char *name;
+    char *name;
     HotplugHandler *hotplug_handler;
     int max_index;
     bool realized;
-- 
2.9.0

[Qemu-devel] [PATCH for-2.7 v4 32/36] tests: add qtest_add_data_func_full

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Allows one to specify a destroy function for the test data.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 tests/libqtest.c | 15 +++++++++++++++
 tests/libqtest.h | 17 +++++++++++++++++
 2 files changed, 32 insertions(+)

diff --git a/tests/libqtest.c b/tests/libqtest.c
index eb00f13..9e2d0cd 100644
--- a/tests/libqtest.c
+++ b/tests/libqtest.c
@@ -758,6 +758,21 @@ void qtest_add_func(const char *str, void (*fn)(void))
     g_free(path);
 }
 
+void qtest_add_data_func_full(const char *str, void *data,
+                              void (*fn)(const void *),
+                              GDestroyNotify data_free_func)
+{
+    gchar *path = g_strdup_printf("/%s/%s", qtest_get_arch(), str);
+#if GLIB_CHECK_VERSION(2, 34, 0)
+    g_test_add_data_func_full(path, data, fn, data_free_func);
+#else
+    /* back-compat casts, remove this once we can require new-enough glib */
+    g_test_add_vtable(path, 0, data, NULL,
+                      (GTestFixtureFunc) fn, (GTestFixtureFunc) data_free_func);
+#endif
+    g_free(path);
+}
+
 void qtest_add_data_func(const char *str, const void *data,
                          void (*fn)(const void *))
 {
diff --git a/tests/libqtest.h b/tests/libqtest.h
index 37f37ad..d2b4853 100644
--- a/tests/libqtest.h
+++ b/tests/libqtest.h
@@ -425,6 +425,23 @@ void qtest_add_func(const char *str, void (*fn)(void));
 void qtest_add_data_func(const char *str, const void *data,
                          void (*fn)(const void *));
 
+/**
+ * qtest_add_data_func_full:
+ * @str: Test case path.
+ * @data: Test case data
+ * @fn: Test case function
+ * @data_free_func: GDestroyNotify for data
+ *
+ * Add a GTester testcase with the given name, data and function.
+ * The path is prefixed with the architecture under test, as
+ * returned by qtest_get_arch().
+ *
+ * @data is passed to @data_free_func() on test completion.
+ */
+void qtest_add_data_func_full(const char *str, void *data,
+                              void (*fn)(const void *),
+                              GDestroyNotify data_free_func);
+
 /**
  * qtest_add:
  * @testpath: Test case path
-- 
2.9.0

[Qemu-devel] [PATCH for-2.7 v4 33/36] tests: pc-cpu-test leaks fixes

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

The path is allocated and should be freed.

The qmp response should be unref, but then 'machine' must be duplicated.

Use a destroy function for the PCTestData.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 tests/pc-cpu-test.c | 24 +++++++++++++++++++-----
 1 file changed, 19 insertions(+), 5 deletions(-)

diff --git a/tests/pc-cpu-test.c b/tests/pc-cpu-test.c
index 4428cea..c3a2633 100644
--- a/tests/pc-cpu-test.c
+++ b/tests/pc-cpu-test.c
@@ -14,7 +14,7 @@
 #include "qapi/qmp/types.h"
 
 struct PCTestData {
-    const char *machine;
+    char *machine;
     const char *cpu_model;
     unsigned sockets;
     unsigned cores;
@@ -71,6 +71,14 @@ static void test_pc_without_cpu_add(gconstpointer data)
     g_free(args);
 }
 
+static void test_data_free(gpointer data)
+{
+    PCTestData *pc = data;
+
+    g_free(pc->machine);
+    g_free(pc);
+}
+
 static void add_pc_test_cases(void)
 {
     QDict *response, *minfo;
@@ -78,7 +86,8 @@ static void add_pc_test_cases(void)
     const QListEntry *p;
     QObject *qobj;
     QString *qstr;
-    const char *mname, *path;
+    const char *mname;
+    char *path;
     PCTestData *data;
 
     qtest_start("-machine none");
@@ -99,7 +108,7 @@ static void add_pc_test_cases(void)
             continue;
         }
         data = g_malloc(sizeof(PCTestData));
-        data->machine = mname;
+        data->machine = g_strdup(mname);
         data->cpu_model = "Haswell"; /* 1.3+ theoretically */
         data->sockets = 1;
         data->cores = 3;
@@ -119,14 +128,19 @@ static void add_pc_test_cases(void)
             path = g_strdup_printf("cpu/%s/init/%ux%ux%u&maxcpus=%u",
                                    mname, data->sockets, data->cores,
                                    data->threads, data->maxcpus);
-            qtest_add_data_func(path, data, test_pc_without_cpu_add);
+            qtest_add_data_func_full(path, data, test_pc_without_cpu_add,
+                                     test_data_free);
+            g_free(path);
         } else {
             path = g_strdup_printf("cpu/%s/add/%ux%ux%u&maxcpus=%u",
                                    mname, data->sockets, data->cores,
                                    data->threads, data->maxcpus);
-            qtest_add_data_func(path, data, test_pc_with_cpu_add);
+            qtest_add_data_func_full(path, data, test_pc_with_cpu_add,
+                                     test_data_free);
+            g_free(path);
         }
     }
+    QDECREF(response);
     qtest_end();
 }
 
-- 
2.9.0

[Qemu-devel] [PATCH for-2.7 v4 34/36] tests: fix rsp leak in postcopy-test

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

In all cases, even when the dict doesn't contain 'ram', the qmp response
must be unref.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 tests/postcopy-test.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/postcopy-test.c b/tests/postcopy-test.c
index 229e9e9..bf4e579 100644
--- a/tests/postcopy-test.c
+++ b/tests/postcopy-test.c
@@ -260,8 +260,8 @@ static uint64_t get_migration_pass(void)
     } else {
         rsp_ram = qdict_get_qdict(rsp_return, "ram");
         result = qdict_get_try_int(rsp_ram, "dirty-sync-count", 0);
-        QDECREF(rsp);
     }
+    QDECREF(rsp);
     return result;
 }
 
-- 
2.9.0

[Qemu-devel] [PATCH for-2.7 v4 35/36] ahci: fix sglist leak on retry

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

ahci-test /x86_64/ahci/io/dma/lba28/retry triggers the following leak:

Direct leak of 16 byte(s) in 1 object(s) allocated from:
    #0 0x7fc4b2a25e20 in malloc (/lib64/libasan.so.3+0xc6e20)
    #1 0x7fc4993bce58 in g_malloc (/lib64/libglib-2.0.so.0+0x4ee58)
    #2 0x556a187d4b34 in ahci_populate_sglist hw/ide/ahci.c:896
    #3 0x556a187d8237 in ahci_dma_prepare_buf hw/ide/ahci.c:1367
    #4 0x556a187b5a1a in ide_dma_cb hw/ide/core.c:844
    #5 0x556a187d7eec in ahci_start_dma hw/ide/ahci.c:1333
    #6 0x556a187b650b in ide_start_dma hw/ide/core.c:921
    #7 0x556a187b61e6 in ide_sector_start_dma hw/ide/core.c:911
    #8 0x556a187b9e26 in cmd_write_dma hw/ide/core.c:1486
    #9 0x556a187bd519 in ide_exec_cmd hw/ide/core.c:2027
    #10 0x556a187d71c5 in handle_reg_h2d_fis hw/ide/ahci.c:1204
    #11 0x556a187d7681 in handle_cmd hw/ide/ahci.c:1254
    #12 0x556a187d168a in check_cmd hw/ide/ahci.c:510
    #13 0x556a187d0afc in ahci_port_write hw/ide/ahci.c:314
    #14 0x556a187d105d in ahci_mem_write hw/ide/ahci.c:435
    #15 0x556a1831d959 in memory_region_write_accessor /home/elmarco/src/qemu/memory.c:525
    #16 0x556a1831dc35 in access_with_adjusted_size /home/elmarco/src/qemu/memory.c:591
    #17 0x556a18323ce3 in memory_region_dispatch_write /home/elmarco/src/qemu/memory.c:1262
    #18 0x556a1828cf67 in address_space_write_continue /home/elmarco/src/qemu/exec.c:2578
    #19 0x556a1828d20b in address_space_write /home/elmarco/src/qemu/exec.c:2635
    #20 0x556a1828d92b in address_space_rw /home/elmarco/src/qemu/exec.c:2737
    #21 0x556a1828daf7 in cpu_physical_memory_rw /home/elmarco/src/qemu/exec.c:2746
    #22 0x556a183068d3 in cpu_physical_memory_write /home/elmarco/src/qemu/include/exec/cpu-common.h:72
    #23 0x556a18308194 in qtest_process_command /home/elmarco/src/qemu/qtest.c:382
    #24 0x556a18309999 in qtest_process_inbuf /home/elmarco/src/qemu/qtest.c:573
    #25 0x556a18309a4a in qtest_read /home/elmarco/src/qemu/qtest.c:585
    #26 0x556a18598b85 in qemu_chr_be_write_impl /home/elmarco/src/qemu/qemu-char.c:387
    #27 0x556a18598c52 in qemu_chr_be_write /home/elmarco/src/qemu/qemu-char.c:399
    #28 0x556a185a2afa in tcp_chr_read /home/elmarco/src/qemu/qemu-char.c:2902
    #29 0x556a18cbaf52 in qio_channel_fd_source_dispatch io/channel-watch.c:84

Follow John Snow recommendation:
  Everywhere else ncq_err is used, it is accompanied by a list cleanup
  except for ncq_cb, which is the case you are fixing here.

  Move the sglist destruction inside of ncq_err and then delete it from
  the other two locations to keep it tidy.

  Call dma_buf_commit in ide_dma_cb after the early return. Though, this
  is also a little wonky because this routine does more than clear the
  list, but it is at the moment the centralized "we're done with the
  sglist" function and none of the other side effects that occur in
  dma_buf_commit will interfere with the reset that occurs from
  ide_restart_bh, I think

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
---
 hw/ide/ahci.c | 3 +--
 hw/ide/core.c | 1 +
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index 6defeed..f3438ad 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -919,6 +919,7 @@ static void ncq_err(NCQTransferState *ncq_tfs)
     ide_state->error = ABRT_ERR;
     ide_state->status = READY_STAT | ERR_STAT;
     ncq_tfs->drive->port_regs.scr_err |= (1 << ncq_tfs->tag);
+    qemu_sglist_destroy(&ncq_tfs->sglist);
     ncq_tfs->used = 0;
 }
 
@@ -1025,7 +1026,6 @@ static void execute_ncq_command(NCQTransferState *ncq_tfs)
     default:
         DPRINTF(port, "error: unsupported NCQ command (0x%02x) received\n",
                 ncq_tfs->cmd);
-        qemu_sglist_destroy(&ncq_tfs->sglist);
         ncq_err(ncq_tfs);
     }
 }
@@ -1092,7 +1092,6 @@ static void process_ncq_command(AHCIState *s, int port, uint8_t *cmd_fis,
         error_report("ahci: PRDT length for NCQ command (0x%zx) "
                      "is smaller than the requested size (0x%zx)",
                      ncq_tfs->sglist.size, size);
-        qemu_sglist_destroy(&ncq_tfs->sglist);
         ncq_err(ncq_tfs);
         ahci_trigger_irq(ad->hba, ad, PORT_IRQ_OVERFLOW);
         return;
diff --git a/hw/ide/core.c b/hw/ide/core.c
index f9c8162..b0e42a6 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -824,6 +824,7 @@ static void ide_dma_cb(void *opaque, int ret)
     if (ret < 0) {
         if (ide_handle_rw_error(s, -ret, ide_dma_cmd_to_retry(s->dma_cmd))) {
             s->bus->dma->aiocb = NULL;
+            dma_buf_commit(s, 0);
             return;
         }
     }
-- 
2.9.0

[Qemu-devel] [PATCH for-2.7 v4 36/36] tests: fix postcopy-test leaks

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

A few strings are allocated and never freed.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 tests/postcopy-test.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/tests/postcopy-test.c b/tests/postcopy-test.c
index bf4e579..41ed1a9 100644
--- a/tests/postcopy-test.c
+++ b/tests/postcopy-test.c
@@ -176,6 +176,7 @@ static void wait_for_serial(const char *side)
     int started = (strcmp(side, "src_serial") == 0 &&
                    strcmp(arch, "ppc64") == 0) ? 0 : 1;
 
+    g_free(serialpath);
     do {
         int readvalue = fgetc(serialfile);
 
@@ -203,7 +204,6 @@ static void wait_for_serial(const char *side)
         case 'B':
             /* It's alive! */
             fclose(serialfile);
-            g_free(serialpath);
             return;
 
         case EOF:
@@ -350,6 +350,7 @@ static void cleanup(const char *filename)
     char *path = g_strdup_printf("%s/%s", tmpfs, filename);
 
     unlink(path);
+    g_free(path);
 }
 
 static void test_migrate(void)
@@ -394,6 +395,8 @@ static void test_migrate(void)
         g_assert_not_reached();
     }
 
+    g_free(bootpath);
+
     from = qtest_start(cmd_src);
     g_free(cmd_src);
 
-- 
2.9.0

Re: [Qemu-devel] [PATCH for-2.7 v4 00/36] Various memory leak fixes

[Daniel P. Berrange]

On Fri, Aug 05, 2016 at 12:23:45PM +0400, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> Hi,
> 
> Thanks to AddressSanitizer (ASAN), I found a number of direct leaks
> worth fixing. Note that there are probably many indirect leaks left (I
> am adding some here), I haven't investigated much yet.
> 
> There are still a number of direct leaks remaining, in particular in
> the tests, but my libc doesn't give me good backtraces.
> 
> In order to easily switch to asan-enabled build, I make use of make
> CFLAGS argument, which is why the first patch is also there.

FWIW, you could probably use  ./configure --extra-cflags="....." which is
the currently supported way to inject extra custom flags into $CFLAGS.


Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH for-2.7 v4 19/36] char: free the tcp connection data when closing

[Paolo Bonzini]

On 05/08/2016 10:24, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> Make sure the connection data got freed when closing the chardev, to
> avoid leaks. Introduce tcp_chr_free_connection() to clean all connection
> related data, and move some tcp_chr_close() clean-ups there.
> 
> (while at it, set write_msgfds_num to 0 when clearing array in
> tcp_set_msgfds())
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  qemu-char.c | 50 +++++++++++++++++++++++++++++++-------------------
>  1 file changed, 31 insertions(+), 19 deletions(-)
> 
> diff --git a/qemu-char.c b/qemu-char.c
> index a50b8fb..f20d385 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -2763,6 +2763,7 @@ static int tcp_set_msgfds(CharDriverState *chr, int *fds, int num)
>      /* clear old pending fd array */
>      g_free(s->write_msgfds);
>      s->write_msgfds = NULL;
> +    s->write_msgfds_num = 0;
>  
>      if (!s->connected ||
>          !qio_channel_has_feature(s->ioc,
> @@ -2843,19 +2844,24 @@ static GSource *tcp_chr_add_watch(CharDriverState *chr, GIOCondition cond)
>      return qio_channel_create_watch(s->ioc, cond);
>  }
>  
> -static void tcp_chr_disconnect(CharDriverState *chr)
> +static void tcp_chr_free_connection(CharDriverState *chr)
>  {
>      TCPCharDriver *s = chr->opaque;
> +    int i;
>  
>      if (!s->connected) {
>          return;
>      }
>  
> -    s->connected = 0;
> -    if (s->listen_ioc) {
> -        s->listen_tag = qio_channel_add_watch(
> -            QIO_CHANNEL(s->listen_ioc), G_IO_IN, tcp_chr_accept, chr, NULL);
> +    if (s->read_msgfds_num) {
> +        for (i = 0; i < s->read_msgfds_num; i++) {
> +            close(s->read_msgfds[i]);
> +        }
> +        g_free(s->read_msgfds);
> +        s->read_msgfds = NULL;
> +        s->read_msgfds_num = 0;
>      }
> +
>      tcp_set_msgfds(chr, NULL, 0);
>      remove_fd_in_watch(chr);
>      object_unref(OBJECT(s->sioc));
> @@ -2863,6 +2869,24 @@ static void tcp_chr_disconnect(CharDriverState *chr)
>      object_unref(OBJECT(s->ioc));
>      s->ioc = NULL;
>      g_free(chr->filename);
> +    chr->filename = NULL;
> +    s->connected = 0;
> +}
> +
> +static void tcp_chr_disconnect(CharDriverState *chr)
> +{
> +    TCPCharDriver *s = chr->opaque;
> +
> +    if (!s->connected) {
> +        return;
> +    }
> +
> +    tcp_chr_free_connection(chr);
> +
> +    if (s->listen_ioc) {
> +        s->listen_tag = qio_channel_add_watch(
> +            QIO_CHANNEL(s->listen_ioc), G_IO_IN, tcp_chr_accept, chr, NULL);
> +    }
>      chr->filename = SocketAddress_to_str("disconnected:", s->addr,
>                                           s->is_listen, s->is_telnet);
>      qemu_chr_be_event(chr, CHR_EVENT_CLOSED);
> @@ -3177,17 +3201,14 @@ int qemu_chr_wait_connected(CharDriverState *chr, Error **errp)
>  static void tcp_chr_close(CharDriverState *chr)
>  {
>      TCPCharDriver *s = chr->opaque;
> -    int i;
> +
> +    tcp_chr_free_connection(chr);
>  
>      if (s->reconnect_timer) {
>          g_source_remove(s->reconnect_timer);
>          s->reconnect_timer = 0;
>      }
>      qapi_free_SocketAddress(s->addr);
> -    remove_fd_in_watch(chr);
> -    if (s->ioc) {
> -        object_unref(OBJECT(s->ioc));
> -    }
>      if (s->listen_tag) {
>          g_source_remove(s->listen_tag);
>          s->listen_tag = 0;
> @@ -3195,18 +3216,9 @@ static void tcp_chr_close(CharDriverState *chr)
>      if (s->listen_ioc) {
>          object_unref(OBJECT(s->listen_ioc));
>      }
> -    if (s->read_msgfds_num) {
> -        for (i = 0; i < s->read_msgfds_num; i++) {
> -            close(s->read_msgfds[i]);
> -        }
> -        g_free(s->read_msgfds);
> -    }
>      if (s->tls_creds) {
>          object_unref(OBJECT(s->tls_creds));
>      }
> -    if (s->write_msgfds_num) {
> -        g_free(s->write_msgfds);
> -    }
>      g_free(s);
>      qemu_chr_be_event(chr, CHR_EVENT_CLOSED);
>  }
> 

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Re: [Qemu-devel] [PATCH for-2.7 v4 03/36] glib-compat: add g_(s)list_free_full()

[Markus Armbruster]

marcandre.lureau@redhat.com writes:

> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Those functions are only available since glib 2.28.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>

Reviewed-by: Markus Armbruster <armbru@redhat.com>

Re: [Qemu-devel] [PATCH for-2.7 v4 04/36] qga: free the whole blacklist

[Markus Armbruster]

Copying maintainer.

marcandre.lureau@redhat.com writes:

> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Free the config blacklist list, not just the elements. Do it so in the
> more appropriate function config_free().
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  qga/main.c | 7 +------
>  1 file changed, 1 insertion(+), 6 deletions(-)
>
> diff --git a/qga/main.c b/qga/main.c
> index 4c3b2c7..bb48214 100644
> --- a/qga/main.c
> +++ b/qga/main.c
> @@ -1175,6 +1175,7 @@ static void config_free(GAConfig *config)
>  #ifdef CONFIG_FSFREEZE
>      g_free(config->fsfreeze_hook);
>  #endif
> +    g_list_free_full(config->blacklist, g_free);
>      g_free(config);
>  }
>  
> @@ -1310,11 +1311,6 @@ static int run_agent(GAState *s, GAConfig *config)
>      return EXIT_SUCCESS;
>  }
>  
> -static void free_blacklist_entry(gpointer entry, gpointer unused)
> -{
> -    g_free(entry);
> -}
> -
>  int main(int argc, char **argv)
>  {
>      int ret = EXIT_SUCCESS;
> @@ -1379,7 +1375,6 @@ end:
>      if (s->channel) {
>          ga_channel_free(s->channel);
>      }
> -    g_list_foreach(config->blacklist, free_blacklist_entry, NULL);
>      g_free(s->pstate_filepath);
>      g_free(s->state_filepath_isfrozen);

Re: [Qemu-devel] [PATCH for-2.7 v4 05/36] qga: free remaining leaking state

[Markus Armbruster]

Copying maintainer.

marcandre.lureau@redhat.com writes:

> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  qga/guest-agent-command-state.c | 6 ++++++
>  qga/guest-agent-core.h          | 1 +
>  qga/main.c                      | 6 ++++++
>  3 files changed, 13 insertions(+)
>
> diff --git a/qga/guest-agent-command-state.c b/qga/guest-agent-command-state.c
> index 4de229c..e609d32 100644
> --- a/qga/guest-agent-command-state.c
> +++ b/qga/guest-agent-command-state.c
> @@ -71,3 +71,9 @@ GACommandState *ga_command_state_new(void)
>      cs->groups = NULL;
>      return cs;
>  }
> +
> +void ga_command_state_free(GACommandState *cs)
> +{
> +    g_slist_free_full(cs->groups, g_free);
> +    g_free(cs);
> +}
> diff --git a/qga/guest-agent-core.h b/qga/guest-agent-core.h
> index 0a49516..63e9d39 100644
> --- a/qga/guest-agent-core.h
> +++ b/qga/guest-agent-core.h
> @@ -28,6 +28,7 @@ void ga_command_state_add(GACommandState *cs,
>  void ga_command_state_init_all(GACommandState *cs);
>  void ga_command_state_cleanup_all(GACommandState *cs);
>  GACommandState *ga_command_state_new(void);
> +void ga_command_state_free(GACommandState *cs);
>  bool ga_logging_enabled(GAState *s);
>  void ga_disable_logging(GAState *s);
>  void ga_enable_logging(GAState *s);
> diff --git a/qga/main.c b/qga/main.c
> index bb48214..0b9d04e 100644
> --- a/qga/main.c
> +++ b/qga/main.c
> @@ -1371,6 +1371,8 @@ int main(int argc, char **argv)
>  end:
>      if (s->command_state) {
>          ga_command_state_cleanup_all(s->command_state);
> +        ga_command_state_free(s->command_state);
> +        json_message_parser_destroy(&s->parser);
>      }
>      if (s->channel) {
>          ga_channel_free(s->channel);
> @@ -1383,6 +1385,10 @@ end:
>      }
>  
>      config_free(config);
> +    if (s->main_loop) {
> +        g_main_loop_unref(s->main_loop);
> +    }
> +    g_free(s);
>  
>      return ret;
>  }

Re: [Qemu-devel] [PATCH for-2.7 v4 32/36] tests: add qtest_add_data_func_full

[Markus Armbruster]

marcandre.lureau@redhat.com writes:

> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Allows one to specify a destroy function for the test data.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>

Since Eric's okay with the function type punning in the compatibility
code for old GLib, I refrain from rocking the boat, and just say
Reviewed-by: Markus Armbruster <armbru@redhat.com>

Re: [Qemu-devel] [PATCH for-2.7 v4 05/36] qga: free remaining leaking state

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 599 bytes --]

On 08/05/2016 05:26 AM, Markus Armbruster wrote:
> Copying maintainer.
> 
> marcandre.lureau@redhat.com writes:
> 
>> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>>
>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>> ---
>>  qga/guest-agent-command-state.c | 6 ++++++
>>  qga/guest-agent-core.h          | 1 +
>>  qga/main.c                      | 6 ++++++
>>  3 files changed, 13 insertions(+)
>>

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH for-2.7 v4 04/36] qga: free the whole blacklist

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 621 bytes --]

On 08/05/2016 05:25 AM, Markus Armbruster wrote:
> Copying maintainer.
> 
> marcandre.lureau@redhat.com writes:
> 
>> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>>
>> Free the config blacklist list, not just the elements. Do it so in the
>> more appropriate function config_free().
>>
>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>> ---
>>  qga/main.c | 7 +------
>>  1 file changed, 1 insertion(+), 6 deletions(-)

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH for-2.7 v4 00/36] Various memory leak fixes

[Marc-André Lureau]

Hi Peter,

All patches have a reviewed-by tag now, Given the mix of domains, I can
send you a pull request. What do you think?

thanks

On Fri, Aug 5, 2016 at 12:26 PM <marcandre.lureau@redhat.com> wrote:

> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Hi,
>
> Thanks to AddressSanitizer (ASAN), I found a number of direct leaks
> worth fixing. Note that there are probably many indirect leaks left (I
> am adding some here), I haven't investigated much yet.
>
> There are still a number of direct leaks remaining, in particular in
> the tests, but my libc doesn't give me good backtraces.
>
> In order to easily switch to asan-enabled build, I make use of make
> CFLAGS argument, which is why the first patch is also there.
>
> v4:
> - document qtest_add_data_func_full seperately
> - add g_(s)list_free_full in glib-compat
> - simplify tcp_chr_close()
> - move dma_buf_commit() down before the early return case
> - dropped a qdist leak patch already applied
>
> v3:
> - add glib<2.34 fallback for g_test_add_data_func_full() using
>   g_test_add_vtable() with an explicit comment
> - add and use a tcp_chr_free_connection(), from tcp_chr_disconnect()
>   (suggested by Paolo)
> - remove bad comment from "free MuxDriver" patch (Eric)
> - replace GFunc casts with a qemu_g_func_free() inline (suggested by Eric)
> - replace CFLAGS override patch with a toplevel CFLAGS filter
>   (suggested by Paolo)
> - drop a patch already merged upstream
> - update reviewed-by tags (28/36)
>
> v2:
> - split "pci-bus: do not allocate and leak bsel" as a seperate patch,
>   since the direct leak looks like a bug
> - hook virtio_input_finalize to the abstract base class
> - use mkdtemp in test-io-channel-command
> - fix pc-cpu-test commit message
> - move the sglist destruction inside of ncq_err, call dma_buf_commit
>   in ide_dma_cb, as suggested by John Snow
> - fix build with glib 2.22
> - add the given reviewed-by tags (22/37, 15 left)
>
> Marc-André Lureau (36):
>   build-sys: fix building with make CFLAGS=.. argument
>   tests: fix test-qga leaks
>   glib-compat: add g_(s)list_free_full()
>   qga: free the whole blacklist
>   qga: free remaining leaking state
>   tests: fix test-cutils leaks
>   tests: fix test-vmstate leaks
>   tests: fix test-iov leaks
>   tests: fix check-qom-interface leaks
>   tests: fix check-qom-proplist leaks
>   tests: fix small leak in test-io-channel-command
>   tests: fix leak in test-string-input-visitor
>   portio: keep references on portio
>   numa: do not leak NumaOptions
>   pc: simplify passing qemu_irq
>   pc: don't leak a20_line
>   machine: use class base init generated name
>   acpi-build: fix array leak
>   char: free the tcp connection data when closing
>   char: free MuxDriver when closing
>   tests: fix qom-test leaks
>   pc: free i8259
>   pc: keep gsi reference
>   ahci: free irqs array
>   sd: free timer
>   qjson: free str
>   virtio-input: free config list
>   ipmi: free extern timer
>   usb: free USBDevice.strings
>   usb: free leaking path
>   bus: simplify name handling
>   tests: add qtest_add_data_func_full
>   tests: pc-cpu-test leaks fixes
>   tests: fix rsp leak in postcopy-test
>   ahci: fix sglist leak on retry
>   tests: fix postcopy-test leaks
>
>  Makefile                          |  3 +-
>  hw/audio/gus.c                    |  9 ++++--
>  hw/audio/sb16.c                   |  4 ++-
>  hw/block/fdc.c                    |  4 ++-
>  hw/char/parallel.c                |  3 +-
>  hw/core/bus.c                     | 21 ++++----------
>  hw/core/machine.c                 |  1 +
>  hw/display/vga-isa.c              |  8 ++++--
>  hw/dma/i8257.c                    |  6 ++--
>  hw/i386/acpi-build.c              |  4 +--
>  hw/i386/pc.c                      |  9 +++---
>  hw/i386/pc_piix.c                 | 17 ++++++------
>  hw/i386/pc_q35.c                  | 15 +++++-----
>  hw/ide/ahci.c                     |  4 +--
>  hw/ide/core.c                     |  7 +++--
>  hw/input/pckbd.c                  |  4 +--
>  hw/input/virtio-input.c           | 11 ++++++++
>  hw/ipmi/ipmi_bmc_extern.c         |  9 ++++++
>  hw/isa/isa-bus.c                  | 14 ++++------
>  hw/sd/sd.c                        |  9 ++++++
>  hw/usb/bus.c                      |  7 +++++
>  hw/usb/desc.c                     |  1 +
>  include/glib-compat.h             | 24 ++++++++++++++++
>  include/hw/boards.h               |  2 +-
>  include/hw/i386/pc.h              |  4 +--
>  include/hw/ide/internal.h         |  2 ++
>  include/hw/isa/i8257.h            |  2 ++
>  include/hw/isa/isa.h              |  5 +++-
>  include/hw/qdev-core.h            |  2 +-
>  migration/qjson.c                 |  1 +
>  numa.c                            | 15 +++++-----
>  pc-bios/optionrom/Makefile        |  2 --
>  qemu-char.c                       | 58
> ++++++++++++++++++++++++++-------------
>  qga/guest-agent-command-state.c   |  6 ++++
>  qga/guest-agent-core.h            |  1 +
>  qga/main.c                        | 13 +++++----
>  tests/check-qom-interface.c       |  1 +
>  tests/check-qom-proplist.c        | 16 +++++++++++
>  tests/libqtest.c                  | 15 ++++++++++
>  tests/libqtest.h                  | 17 ++++++++++++
>  tests/pc-cpu-test.c               | 24 ++++++++++++----
>  tests/postcopy-test.c             |  7 +++--
>  tests/qom-test.c                  |  5 ++--
>  tests/test-cutils.c               | 24 ++++++++++------
>  tests/test-io-channel-command.c   | 20 +++++++++-----
>  tests/test-iov.c                  |  7 +++++
>  tests/test-qga.c                  |  8 +++++-
>  tests/test-string-input-visitor.c |  1 +
>  tests/test-vmstate.c              |  8 ++++--
>  49 files changed, 331 insertions(+), 129 deletions(-)
>
> --
> 2.9.0
>
>
> --
Marc-André Lureau

Re: [Qemu-devel] [PATCH for-2.7 v4 00/36] Various memory leak fixes

[Peter Maydell]

On 5 August 2016 at 19:01, Marc-André Lureau <marcandre.lureau@gmail.com> wrote:
> Hi Peter,
>
> All patches have a reviewed-by tag now, Given the mix of domains, I can send
> you a pull request. What do you think?

36 patches is a lot to be landing on the eve of rc2. How many
of these are actual leaks rather than leaks in test cases
or things that are in theory leaks but of objects that
in practice last the life of the QEMU process anyway ?

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-2.7 v4 00/36] Various memory leak fixes

[Marc-André Lureau]

Hi

On Sun, Aug 7, 2016 at 11:15 PM Peter Maydell <peter.maydell@linaro.org>
wrote:

> On 5 August 2016 at 19:01, Marc-André Lureau <marcandre.lureau@gmail.com>
> wrote:
> > Hi Peter,
> >
> > All patches have a reviewed-by tag now, Given the mix of domains, I can
> send
> > you a pull request. What do you think?
>
> 36 patches is a lot to be landing on the eve of rc2. How many
> of these are actual leaks rather than leaks in test cases
> or things that are in theory leaks but of objects that
> in practice last the life of the QEMU process anyway ?
>

They should be fairly safe, but I don't mind moving it to after the
release. Yes, many of the patches are one-time leaks and for tests.

This would be a possible shortlist for 2.7 of things that could be more
than one-timer though:

numa: do not leak NumaOptions
char: free the tcp connection data when closing
char: free MuxDriver when closing
ahci: free irqs array
qjson: free str
virtio-input: free config list
usb: free USBDevice.strings
usb: free leaking path
ahci: fix sglist leak on retry

thanks
-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH for-2.7 v4 00/36] Various memory leak fixes

[Peter Maydell]

On 7 August 2016 at 20:31, Marc-André Lureau <marcandre.lureau@gmail.com> wrote:
> Hi
>
> On Sun, Aug 7, 2016 at 11:15 PM Peter Maydell <peter.maydell@linaro.org>
> wrote:
>>
>> On 5 August 2016 at 19:01, Marc-André Lureau <marcandre.lureau@gmail.com>
>> wrote:
>> > Hi Peter,
>> >
>> > All patches have a reviewed-by tag now, Given the mix of domains, I can
>> > send
>> > you a pull request. What do you think?
>>
>> 36 patches is a lot to be landing on the eve of rc2. How many
>> of these are actual leaks rather than leaks in test cases
>> or things that are in theory leaks but of objects that
>> in practice last the life of the QEMU process anyway ?
>
>
> They should be fairly safe, but I don't mind moving it to after the release.
> Yes, many of the patches are one-time leaks and for tests.
>
> This would be a possible shortlist for 2.7 of things that could be more than
> one-timer though:
>
> numa: do not leak NumaOptions
> char: free the tcp connection data when closing
> char: free MuxDriver when closing
> ahci: free irqs array
> qjson: free str
> virtio-input: free config list
> usb: free USBDevice.strings
> usb: free leaking path
> ahci: fix sglist leak on retry

That looks like a much shorter list which I'd definitely
be happier with applying.

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-2.7 v4 01/36] build-sys: fix building with make CFLAGS=.. argument

[Paolo Bonzini]

On 05/08/2016 10:23, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> When calling make with a CFLAGS=.. argument, the -g/-O filter is not
> applied, which may result with build failure with ASAN for example. It
> could be solved with an 'override' directive on CFLAGS, but that would
> actually prevent setting different CFLAGS manually.
> 
> Instead, filter the CFLAGS argument from the top-level Makefile (so
> you could still call make with a different CFLAGS argument on a
> rom/Makefile manually)
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Applying this patch for 2.7.

Paolo

> ---
>  Makefile                   | 3 ++-
>  pc-bios/optionrom/Makefile | 2 --
>  2 files changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 0d7647f..50b4b3a 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -225,8 +225,9 @@ dtc/%:
>  $(SUBDIR_RULES): libqemuutil.a libqemustub.a $(common-obj-y) $(qom-obj-y) $(crypto-aes-obj-$(CONFIG_USER_ONLY))
>  
>  ROMSUBDIR_RULES=$(patsubst %,romsubdir-%, $(ROMS))
> +# Only keep -O and -g cflags
>  romsubdir-%:
> -	$(call quiet-command,$(MAKE) $(SUBDIR_MAKEFLAGS) -C pc-bios/$* V="$(V)" TARGET_DIR="$*/",)
> +	$(call quiet-command,$(MAKE) $(SUBDIR_MAKEFLAGS) -C pc-bios/$* V="$(V)" TARGET_DIR="$*/" CFLAGS="$(filter -O% -g%,$(CFLAGS))",)
>  
>  ALL_SUBDIRS=$(TARGET_DIRS) $(patsubst %,pc-bios/%, $(ROMS))
>  
> diff --git a/pc-bios/optionrom/Makefile b/pc-bios/optionrom/Makefile
> index 24e175e..6bab490 100644
> --- a/pc-bios/optionrom/Makefile
> +++ b/pc-bios/optionrom/Makefile
> @@ -24,8 +24,6 @@ QEMU_CFLAGS += $(call cc-option, $(QEMU_CFLAGS), -no-integrated-as)
>  QEMU_CFLAGS += -m32 -include $(SRC_PATH)/pc-bios/optionrom/code16gcc.h
>  endif
>  
> -# Drop gcov and glib flags
> -CFLAGS := $(filter -O% -g%, $(CFLAGS))
>  QEMU_INCLUDES += -I$(SRC_PATH)
>  
>  Wa = -Wa,
>