Re: Help with create_pipe_bidi

[Bruno Haible]

Eric Blake wrote:
> It is due to a bug in gnulib.

Yup. Thanks for committing the test and the fix.

> |  if (pipe_stdin)
> |    if (pipe (ofd) < 0
> |        || (ofd[1] = fd_safer (ofd[1])) < 0)
> 
> This has the effect of setting ofd to {1,5} if exactly one of stdin/stdout 
> were 
> closed.

Indeed, this is a problem. Running "./test-pipe 1", the situation is:
  0 is closed
  1, 2 open
  ifd = { 0, 3 } -> ifd = { 4, 3 }
  ofd = { 0, 5 }
  child: dup2(0,0), dup2(3,1), close(0), close(3), close(5), close(4), execve.
         ^^^^^^^^^             ^^^^^^^^

The close(0) call should have been omitted in this case. But there are a couple
of similarly weird cases. Your fix handles them well.

> Then, in the child: 
> 
> | if (pipe_stdin)
> |   {
> |     close (ofd[0]);
> |     close (ofd[1]);
> |   }
> 
> we blindly closed both descriptors.

That code is OK, it's just the error handling code in the parent process.

> And this gnulib bug means that m4 has a regression in 1.4.13 (which uses 
> create_pipe_in) when compared to 1.4.11 (which used popen) (although in the 
> create_pipe_in case, the bug requires closing 2 of the std descriptors, not 
> just one as in create_pipe_out or create_pipe_bidi):
> 
> $ echo 'esyscmd(echo hi >&2)dnl' > oops.m4
> $ m4-1.4.11 <&- >&- oops.m4
> hi
> $ m4-1.4.13 oops.m4 <&- >&-
> /bin/m4: esyscmd subprocess failed
> $ echo $?
> 0

Strange: On Linux, I don't see this failure. I see

$ m4-1.4.11 oops.m4 <&- >&-
hi
$ m4-1.4.12 oops.m4 <&- >&-
hi
$ m4-1.4.13 oops.m4 <&- >&-
hi

But on Cygwin 1.5.x, I see it as well. And I confirm that your patch fixes it.

I fixed a typo in the module description:


2009-07-18  Bruno Haible  <address@hidden>

        * modules/pipe-tests (Makefile.am): Fix typo.

--- modules/pipe-tests.orig     2009-07-18 17:12:37.000000000 +0200
+++ modules/pipe-tests  2009-07-18 15:53:44.000000000 +0200
@@ -11,4 +11,4 @@
 TESTS += test-pipe.sh
 TESTS_ENVIRONMENT += EXEEXT='@EXEEXT@'
 check_PROGRAMS += test-pipe
-test_closein_LDADD = $(LDADD) @LIBINTL@
+test_pipe_LDADD = $(LDADD) @LIBINTL@