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

socat "PIPE:"" on Windows are named pipes, not fifo path names.

Fixes: commit 68406d10859 ("tests/unit: cleanups for test-io-channel-command")
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 tests/unit/test-io-channel-command.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/tests/unit/test-io-channel-command.c b/tests/unit/test-io-channel-command.c
index 096224962c..e76ef2daaa 100644
--- a/tests/unit/test-io-channel-command.c
+++ b/tests/unit/test-io-channel-command.c
@@ -31,8 +31,12 @@ static char *socat = NULL;
 
 static void test_io_channel_command_fifo(bool async)
 {
+#ifdef WIN32
+    const gchar *fifo = TEST_FIFO;
+#else
     g_autofree gchar *tmpdir = g_dir_make_tmp("qemu-test-io-channel.XXXXXX", NULL);
     g_autofree gchar *fifo = g_build_filename(tmpdir, TEST_FIFO, NULL);
+#endif
     g_autofree gchar *srcargs = g_strdup_printf("%s - PIPE:%s,wronly", socat, fifo);
     g_autofree gchar *dstargs = g_strdup_printf("%s PIPE:%s,rdonly -", socat, fifo);
     g_auto(GStrv) srcargv = g_strsplit(srcargs, " ", -1);
@@ -57,7 +61,9 @@ static void test_io_channel_command_fifo(bool async)
     object_unref(OBJECT(src));
     object_unref(OBJECT(dst));
 
+#ifndef WIN32
     g_rmdir(tmpdir);
+#endif
 }
 
 
-- 
2.39.1

On 29/1/23 19:24, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> socat "PIPE:"" on Windows are named pipes, not fifo path names.
> 
> Fixes: commit 68406d10859 ("tests/unit: cleanups for test-io-channel-command")
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>   tests/unit/test-io-channel-command.c | 6 ++++++
>   1 file changed, 6 insertions(+)

This doesn't apply cleanly on top of c906e6fbaa ("tests/unit: drop hacky
race avoidance in test-io-channel-command").

On 29/01/2023 19.24, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> socat "PIPE:"" on Windows are named pipes, not fifo path names.
> 
> Fixes: commit 68406d10859 ("tests/unit: cleanups for test-io-channel-command")
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>   tests/unit/test-io-channel-command.c | 6 ++++++
>   1 file changed, 6 insertions(+)
> 
> diff --git a/tests/unit/test-io-channel-command.c b/tests/unit/test-io-channel-command.c
> index 096224962c..e76ef2daaa 100644
> --- a/tests/unit/test-io-channel-command.c
> +++ b/tests/unit/test-io-channel-command.c
> @@ -31,8 +31,12 @@ static char *socat = NULL;
>   
>   static void test_io_channel_command_fifo(bool async)
>   {
> +#ifdef WIN32
> +    const gchar *fifo = TEST_FIFO;

Question from a Windows ignorant: Won't this cause a race condition in case 
someone is trying to run tests in parallel (i.e. shouldn't there be a random 
part in the name)? Or are these named pipes local to the current process?

  Thomas

> +#else
>       g_autofree gchar *tmpdir = g_dir_make_tmp("qemu-test-io-channel.XXXXXX", NULL);
>       g_autofree gchar *fifo = g_build_filename(tmpdir, TEST_FIFO, NULL);
> +#endif
>       g_autofree gchar *srcargs = g_strdup_printf("%s - PIPE:%s,wronly", socat, fifo);
>       g_autofree gchar *dstargs = g_strdup_printf("%s PIPE:%s,rdonly -", socat, fifo);
>       g_auto(GStrv) srcargv = g_strsplit(srcargs, " ", -1);
> @@ -57,7 +61,9 @@ static void test_io_channel_command_fifo(bool async)
>       object_unref(OBJECT(src));
>       object_unref(OBJECT(dst));
>   
> +#ifndef WIN32
>       g_rmdir(tmpdir);
> +#endif
>   }
>   
>   

Hi

On Mon, Feb 6, 2023 at 12:09 PM Thomas Huth <thuth@redhat.com> wrote:
>
> On 29/01/2023 19.24, marcandre.lureau@redhat.com wrote:
> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> > socat "PIPE:"" on Windows are named pipes, not fifo path names.
> >
> > Fixes: commit 68406d10859 ("tests/unit: cleanups for test-io-channel-command")
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> >   tests/unit/test-io-channel-command.c | 6 ++++++
> >   1 file changed, 6 insertions(+)
> >
> > diff --git a/tests/unit/test-io-channel-command.c b/tests/unit/test-io-channel-command.c
> > index 096224962c..e76ef2daaa 100644
> > --- a/tests/unit/test-io-channel-command.c
> > +++ b/tests/unit/test-io-channel-command.c
> > @@ -31,8 +31,12 @@ static char *socat = NULL;
> >
> >   static void test_io_channel_command_fifo(bool async)
> >   {
> > +#ifdef WIN32
> > +    const gchar *fifo = TEST_FIFO;
>
> Question from a Windows ignorant: Won't this cause a race condition in case
> someone is trying to run tests in parallel (i.e. shouldn't there be a random
> part in the name)? Or are these named pipes local to the current process?
>

You are right, they are global. Furthermore, socat PIPE: is actually
not using windows named-pipes, at least directly (despite debugging
log saying something like that). It's actually mkfifo() cygwin/newlib
implementation that does some magic (in msys2). It looks like it
creates some kind of "fake" file/links with metadata and actually
creates the named pipe later on demand. So it still maps to some kind
of file, and it looks like create/open races would still be possible,
even if "cygwin" mkfifo was called before socat. The alternative would
be to create pipe() beforehand and handing those fd to socat, perhaps.
I might give it a try.

thanks

>   Thomas
>
> > +#else
> >       g_autofree gchar *tmpdir = g_dir_make_tmp("qemu-test-io-channel.XXXXXX", NULL);
> >       g_autofree gchar *fifo = g_build_filename(tmpdir, TEST_FIFO, NULL);
> > +#endif
> >       g_autofree gchar *srcargs = g_strdup_printf("%s - PIPE:%s,wronly", socat, fifo);
> >       g_autofree gchar *dstargs = g_strdup_printf("%s PIPE:%s,rdonly -", socat, fifo);
> >       g_auto(GStrv) srcargv = g_strsplit(srcargs, " ", -1);
> > @@ -57,7 +61,9 @@ static void test_io_channel_command_fifo(bool async)
> >       object_unref(OBJECT(src));
> >       object_unref(OBJECT(dst));
> >
> > +#ifndef WIN32
> >       g_rmdir(tmpdir);
> > +#endif
> >   }
> >
> >
>
>


-- 
Marc-André Lureau