Re: [Qemu-devel] [PATCH v2 11/37] tests: fix small leak in test-io-channel-command

[Eric Blake]

On 07/28/2016 08:37 AM, address@hidden wrote:
> From: Marc-André Lureau <address@hidden>
> 
> 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 <address@hidden>
> ---
>  tests/test-io-channel-command.c | 20 +++++++++++++-------
>  1 file changed, 13 insertions(+), 7 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));

I'm always wary of side effects inside assert().  But if I recall
correctly, g_assert_nonnull() is unconditional (there's no way like
NDEBUG to completely compile away the side effect), so I guess you're
safe, even though I would have stored the results of mkdtemp() in a
temporary and only then asserted that the temporary is non-null.

Whether or not you clean that up,

Reviewed-by: Eric Blake <address@hidden>

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

signature.asc
Description: OpenPGP digital signature