| Message ID | 20160728143808.13707-12-marcandre.lureau@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 07/28/2016 08:37 AM, marcandre.lureau@redhat.com wrote: > 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> > --- > 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 <eblake@redhat.com>
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); }