Re: [PATCH] tests/qtest/vhost-user-test: Fix memory leaks

[Philippe Mathieu-Daudé]

On 1/16/20 3:07 PM, Thomas Huth wrote:
> Do not allocate resources in case we return early, and make sure
> to free dest_cmdline at the end.
>
> Reported-by: Euler Robot <address@hidden>
> Reported-by: Pan Nengyuan <address@hidden>
> Signed-off-by: Thomas Huth <address@hidden>
> ---
>   Based on a similar patch by Pan Nengyuan:
>    https://lists.gnu.org/archive/html/qemu-devel/2020-01/msg02183.html
>   ... but that was causing some very weird problems in the gitlab CI
>   for unknown reasons. This version now works fine in the gitlab CI.

Odd...

>    
>   tests/qtest/vhost-user-test.c | 11 ++++++++---
>   1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/tests/qtest/vhost-user-test.c b/tests/qtest/vhost-user-test.c
> index 2324b964ad..9ee0f1e4fd 100644
> --- a/tests/qtest/vhost-user-test.c
> +++ b/tests/qtest/vhost-user-test.c
> @@ -707,9 +707,9 @@ static void test_read_guest_mem(void *obj, void *arg, 
> QGuestAllocator *alloc)
>   static void test_migrate(void *obj, void *arg, QGuestAllocator *alloc)
>   {
>       TestServer *s = arg;
> -    TestServer *dest = test_server_new("dest");
> -    GString *dest_cmdline = g_string_new(qos_get_current_command_line());
> -    char *uri = g_strdup_printf("%s%s", "unix:", dest->mig_path);
> +    TestServer *dest;
> +    GString *dest_cmdline;
> +    char *uri;
>       QTestState *to;
>       GSource *source;
>       QDict *rsp;
> @@ -720,6 +720,10 @@ static void test_migrate(void *obj, void *arg, 
> QGuestAllocator *alloc)
>           return;
>       }
>   
> +    dest = test_server_new("dest");
> +    dest_cmdline = g_string_new(qos_get_current_command_line());
> +    uri = g_strdup_printf("%s%s", "unix:", dest->mig_path);
> +
>       size = get_log_size(s);
>       g_assert_cmpint(size, ==, (256 * 1024 * 1024) / (VHOST_LOG_PAGE * 8));
>   
> @@ -778,6 +782,7 @@ static void test_migrate(void *obj, void *arg, QGuestAllocator *alloc)
>       qtest_quit(to);
>       test_server_free(dest);
>       g_free(uri);
> +    g_string_free(dest_cmdline, true);

Simpler, good.

Reviewed-by: Philippe Mathieu-Daudé <address@hidden>

>   }
>   
>   static void wait_for_rings_started(TestServer *s, size_t count)