From: Gökbey Keskin <[email protected]>

Several tests under 27_io open a FIFO with ios_base::in|ios_base::out, which
fopen_mode() maps to "r+", i.e. O_RDWR.  POSIX leaves this undefined:

  O_RDWR
      Open for reading and writing.  The result is undefined if this flag is
      applied to a FIFO.

Linux permits it, so the problem is invisible there, but it is not required.
On a target where opening a FIFO O_RDWR fails, the affected tests deadlock:
the process that issues the O_RDWR open fails to open the FIFO, so its peer
blocks forever in a blocking open() for the opposite direction, because the
FIFO never acquires a second endpoint.

None of these tests needs read-write access.  In every case the process
opening in|out only ever writes.  Opening write-only replaces the O_RDWR open
with the ordinary FIFO rendezvous (child O_WRONLY, parent O_RDONLY, or vice
versa), which is fully specified and introduces no ordering hazard.

Second, separate issue: close/char/9964.cc has not tested PR 9964 since 2004.

PR libstdc++/9964 was fixed by Petur Runolfsson in
r0-48647-g0c45b8e0733816 so that basic_filebuf::close() always closes
the file even if it returns null to indicate an error.

The test as originally committed opened the parent end write-only and asserted
exactly that:

  filebuf* ret = fb.open(name, ios_base::out | ios_base::trunc);
  ...
  ret = fb.close();
  VERIFY( ret == NULL );
  VERIFY( !fb.is_open() );

The write-only open is what makes the flush fail: the child closes the sole
read end, the parent's buffered 'a' meets a FIFO with no readers, SIGPIPE is
ignored, and write() fails with EPIPE.

r0-56237-g6a734d618f8 (2004-02-04, "Correct flags to filebuf::open calls",
the commit that introduced fopen_mode) changed both the mode and the
expectation:

  -  filebuf* ret = fb.open(name, ios_base::out | ios_base::trunc);
  +  filebuf* ret = fb.open(name, ios_base::in | ios_base::out);
  ...
  -  VERIFY( ret == NULL );
  +  VERIFY( ret != NULL );

With O_RDWR the parent holds its own read end, so the write at close succeeds
and the failure path is never entered.  The test now passes whether or not the
PR 9964 fix is present, i.e. it is a dead regression test.

So the fix for this file is to restore the original write-only open and the
original close() expectation, which also removes its O_RDWR dependency.

libstdc++-v3/ChangeLog:

        PR libstdc++/127424
        * testsuite/27_io/basic_filebuf/close/char/9964.cc: Only open
        filebuf for output and expect close() to return null.
        * testsuite/27_io/basic_filebuf/seekoff/char/26777.cc: Only open
        filebuf for output.
        * testsuite/27_io/basic_filebuf/showmanyc/char/9533-1.cc:
        Likewise.
        * testsuite/27_io/basic_filebuf/underflow/char/10097.cc:
        Likewise.
        * testsuite/27_io/objects/char/7.cc: Likewise.
        * testsuite/27_io/objects/char/9661-1.cc: Only open FILE for
        output.
---

Tested x86_64-linux. Pushed to trunk.

I'm ambivalent whether to backport this one.

 .../testsuite/27_io/basic_filebuf/close/char/9964.cc       | 7 +++++--
 .../testsuite/27_io/basic_filebuf/seekoff/char/26777.cc    | 2 +-
 .../testsuite/27_io/basic_filebuf/showmanyc/char/9533-1.cc | 2 +-
 .../testsuite/27_io/basic_filebuf/underflow/char/10097.cc  | 2 +-
 libstdc++-v3/testsuite/27_io/objects/char/7.cc             | 2 +-
 libstdc++-v3/testsuite/27_io/objects/char/9661-1.cc        | 2 +-
 6 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/libstdc++-v3/testsuite/27_io/basic_filebuf/close/char/9964.cc 
b/libstdc++-v3/testsuite/27_io/basic_filebuf/close/char/9964.cc
index 84b406ac0727..e34eea5923b8 100644
--- a/libstdc++-v3/testsuite/27_io/basic_filebuf/close/char/9964.cc
+++ b/libstdc++-v3/testsuite/27_io/basic_filebuf/close/char/9964.cc
@@ -64,15 +64,18 @@ bool test_07()
     }
   
   filebuf fb;
-  filebuf* ret = fb.open(name, ios_base::in | ios_base::out);
+  filebuf* ret = fb.open(name, ios_base::out);
   test &= bool( ret != 0 );
   test &= bool( fb.is_open() );
   s1.signal();
   s2.wait();
   fb.sputc('a');
 
+  // The child has closed the only read end of the fifo, so flushing the
+  // pending 'a' fails with EPIPE.  close() must report that failure by
+  // returning null, but must close the file all the same.
   ret = fb.close();
-  test &= bool( ret != 0 );
+  test &= bool( ret == 0 );
   test &= bool( !fb.is_open() );
 
   return test;
diff --git a/libstdc++-v3/testsuite/27_io/basic_filebuf/seekoff/char/26777.cc 
b/libstdc++-v3/testsuite/27_io/basic_filebuf/seekoff/char/26777.cc
index 1931db09ad04..c7c091e4999e 100644
--- a/libstdc++-v3/testsuite/27_io/basic_filebuf/seekoff/char/26777.cc
+++ b/libstdc++-v3/testsuite/27_io/basic_filebuf/seekoff/char/26777.cc
@@ -54,7 +54,7 @@ bool test01()
   if (child == 0)
     {
       filebuf fbout;
-      fbout.open(name, ios_base::in | ios_base::out);
+      fbout.open(name, ios_base::out);
       test &= bool( fbout.is_open() );
       fbout.sputn("Whatever", 8);
       fbout.pubsync();
diff --git 
a/libstdc++-v3/testsuite/27_io/basic_filebuf/showmanyc/char/9533-1.cc 
b/libstdc++-v3/testsuite/27_io/basic_filebuf/showmanyc/char/9533-1.cc
index 57b234515f09..9a42cadacc11 100644
--- a/libstdc++-v3/testsuite/27_io/basic_filebuf/showmanyc/char/9533-1.cc
+++ b/libstdc++-v3/testsuite/27_io/basic_filebuf/showmanyc/char/9533-1.cc
@@ -55,7 +55,7 @@ void test_01()
   else if (fval == 0)
     {
       filebuf ofbuf;
-      ofbuf.open(name, ios_base::in|ios_base::out);
+      ofbuf.open(name, ios_base::out);
       VERIFY( ofbuf.is_open() );
       sleep(1);
 
diff --git a/libstdc++-v3/testsuite/27_io/basic_filebuf/underflow/char/10097.cc 
b/libstdc++-v3/testsuite/27_io/basic_filebuf/underflow/char/10097.cc
index c320b99920bd..eb117bf736ed 100644
--- a/libstdc++-v3/testsuite/27_io/basic_filebuf/underflow/char/10097.cc
+++ b/libstdc++-v3/testsuite/27_io/basic_filebuf/underflow/char/10097.cc
@@ -74,7 +74,7 @@ bool test16()
   else if (fval == 0)
     {
       filebuf fbout;
-      fbout.open(name, ios_base::in|ios_base::out);
+      fbout.open(name, ios_base::out);
       test &= bool( fbout.is_open() );
       fbout.sputn("0123456789", 10);
       fbout.pubsync();
diff --git a/libstdc++-v3/testsuite/27_io/objects/char/7.cc 
b/libstdc++-v3/testsuite/27_io/objects/char/7.cc
index 2edcf56f7889..5a60e1df2b21 100644
--- a/libstdc++-v3/testsuite/27_io/objects/char/7.cc
+++ b/libstdc++-v3/testsuite/27_io/objects/char/7.cc
@@ -55,7 +55,7 @@ bool test07()
   if (child == 0)
     {
       filebuf fbout;
-      fbout.open(name, ios_base::in|ios_base::out);
+      fbout.open(name, ios_base::out);
       test &= bool( fbout.is_open() );
       s1.wait();
       cout.rdbuf(&fbout);
diff --git a/libstdc++-v3/testsuite/27_io/objects/char/9661-1.cc 
b/libstdc++-v3/testsuite/27_io/objects/char/9661-1.cc
index 9374685969db..ba7f55c3bf33 100644
--- a/libstdc++-v3/testsuite/27_io/objects/char/9661-1.cc
+++ b/libstdc++-v3/testsuite/27_io/objects/char/9661-1.cc
@@ -55,7 +55,7 @@ bool test01()
 
   if (child == 0)
     {
-      FILE* file = fopen(name, "r+");
+      FILE* file = fopen(name, "w");
       test &= bool( file != 0 );
       fputs("Whatever\n", file);
       fflush(file);
-- 
2.55.0

Reply via email to