@samitouri / QOSamiQemu / commits / 93ed7d3303

tests/qtest/qos-test: Plug a couple of leaks

The walk_path() function of qos-test.c, which walks the graph and adds tests to the test suite uses GLib's g_test_add_data_func_full() function: g_test_add_data_func_full (const char *testpath, gpointer test_data, GTestDataFunc test_func, GDestroyNotify data_free_func) Despite GLib's documentation stating that @data_free_func is a destructor for @test_data, this is not the case. The destructor is supposed to be paired with a constructor, which GLib only accepts via g_test_create_case(). Providing externally allocated data plus a destructor function only works if the test is guaranteed to execute, otherwise the test_data is never deallocated. Due to how subprocessess are implemented in qos-test, each test gets added twice and an extra test gets added per subprocess. In a regular run, the extra subprocess will not be executed and in a single test run (-p), none of the other tests will be executed (+1 per subprocess), leaking 'path_vec' and 'subprocess_path'. Fix this by storing all the path vectors in a list and freeing them all at the end of the program (including subprocess invocations) and moving the allocation of 'subprocess_path' into run_one_subprocess(). While here add some documentation explaining why the graph needs to be walked twice and tests re-added. Signed-off-by: Fabiano Rosas <farosas@suse.de> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Message-id: 20260302092225.4088227-10-peter.maydell@linaro.org [PMM: rebased; rewrote the comment in main() a bit to account for the if (g_test_subprocess()) block it was previously inside no longer being present. ] Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Fabiano Rosas committed Mar 6, 2026 at 09:01 UTC 93ed7d330321dca483cd4a68fc4db9af4fa1e03e
1 file changed +26 -10
tests/qtest/qos-test.c
+26 -10
@@ -31,6 +31,7 @@
31 #include "libqos/qos_external.h"
32
33 static char *old_path;
34 +static GSList *path_vecs;
35
36
37 /**
@@ -182,11 +183,16 @@ static void run_one_test(const void *arg)
183
184 static void subprocess_run_one_test(const void *arg)
185 {
185 - const gchar *path = arg;
186 - g_test_trap_subprocess(path, 180 * G_USEC_PER_SEC,
186 + char **path_vec = (char **) arg;
187 + gchar *path = g_strjoinv("/", path_vec + 1);
188 + gchar *subprocess_path = g_strdup_printf("/%s/subprocess", path);
189 +
190 + g_test_trap_subprocess(subprocess_path, 180 * G_USEC_PER_SEC,
191 G_TEST_SUBPROCESS_INHERIT_STDOUT |
192 G_TEST_SUBPROCESS_INHERIT_STDERR);
193 g_test_trap_assert_passed();
194 + g_free(path);
195 + g_free(subprocess_path);
196 }
197
198 static void destroy_pathv(void *arg)
@@ -238,6 +244,7 @@ static void walk_path(QOSGraphNode *orig_path, int len)
244 GString *cmd_line = g_string_new("");
245 GString *cmd_line2 = g_string_new("");
246
247 + path_vecs = g_slist_append(path_vecs, path_vec);
248 path = qos_graph_get_node(node_name); /* root */
249 node_name = qos_graph_edge_get_dest(path->path_edge); /* machine name */
250
@@ -297,15 +304,15 @@ static void walk_path(QOSGraphNode *orig_path, int len)
304 path_vec[0] = g_string_free(cmd_line, false);
305
306 if (path->u.test.subprocess) {
300 - gchar *subprocess_path = g_strdup_printf("/%s/%s/subprocess",
301 - qtest_get_arch(), path_str);
302 - qtest_add_data_func_full(path_str, subprocess_path,
303 - subprocess_run_one_test, g_free);
304 - g_test_add_data_func_full(subprocess_path, path_vec,
305 - run_one_test, destroy_pathv);
307 + gchar *subprocess_path = g_strdup_printf("%s/%s", path_str,
308 + "subprocess");
309 +
310 + qtest_add_data_func(path_str, path_vec, subprocess_run_one_test);
311 + qtest_add_data_func(subprocess_path, path_vec, run_one_test);
312 +
313 + g_free(subprocess_path);
314 } else {
307 - qtest_add_data_func_full(path_str, path_vec,
308 - run_one_test, destroy_pathv);
315 + qtest_add_data_func(path_str, path_vec, run_one_test);
316 }
317
318 g_free(path_str);
@@ -340,6 +347,14 @@ int main(int argc, char **argv, char** envp)
347 module_call_init(MODULE_INIT_LIBQOS);
348 qos_set_machines_devices_available();
349
350 + /*
351 + * Even if this invocation was done to run a single test in a
352 + * subprocess (i.e. g_test_subprocess() is true), gtester doesn't
353 + * expose the test name, so w still need to execute the whole
354 + * thing as normal, including walking the QOS graph to add all
355 + * the tests, in order for g_test_run() to find the one /subprocess
356 + * test that it is going to execute.
357 + */
358 qos_graph_foreach_test_path(walk_path);
359 if (g_test_verbose()) {
360 qos_dump_graph();
@@ -348,5 +363,6 @@ int main(int argc, char **argv, char** envp)
363 qtest_end();
364 qos_graph_destroy();
365 g_free(old_path);
366 + g_slist_free_full(path_vecs, (GDestroyNotify)destroy_pathv);
367 return 0;
368 }