Re: [PATCH v2 2/9] tests: fix test-io-channel-command on win32

[Marc-André Lureau]

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