38d96942 missed a side-effect in PerlIO_open flags parsing.

The mode++ was essential in allowing 'rb' for the PerlIO_open() flags.
Without the mode++ the 'b' was left unprocessed and this caused
the oflags to become bogus.

Compress::Bzip2 caught this: https://rt.perl.org/Ticket/Display.html?id=122012
(also Unicode::Map8, Text::Scan, and otehrs)

While doing this, realized that for the "O_BINARY versus O_TEXT" it's
probably the clearest to test for the non-zero-ness of those two flags.

(Is there any "unit testing" of PerlIO? In this case it would be:
 PerlIO_open -> PerlIO_openn -> PerlIOBuf_open -> PerlIOUnix_open ->
 PerlIOUnix_oflags with mode of "rb")

1 files changed, 31 insertions, 12 deletions

diff --git a/perlio.c b/perlio.c
index c4ace6876d..90622c1e6e 100644
--- a/perlio.c
+++ b/perlio.c
@@ -199,10 +199,12 @@ PerlIO_intmode2str(int rawmode, char *mode, int *writing)
 	    mode[ix++] = '+';
 	}
     }
-#ifdef PERLIO_BINARY_AND_TEXT_DIFFERENT_AND_EFFECTIVE
+#if O_BINARY != 0
+    /* Unless O_BINARY is different from zero, bit-and:ing
+     * with it won't do much good. */
     if (rawmode & O_BINARY)
 	mode[ix++] = 'b';
-#endif
+# endif
     mode[ix] = '\0';
     return ptype;
 }
@@ -2529,25 +2531,42 @@ PerlIOUnix_oflags(const char *mode)
 	    oflags |= O_WRONLY;
 	break;
     }
-#ifdef PERLIO_BINARY_AND_TEXT_DIFFERENT_AND_EFFECTIVE
-    if (*mode == 'b') {
-	oflags |= O_BINARY;
+
+    /* XXX TODO: PerlIO_open() test that exercises 'rb' and 'rt'. */
+
+    /* Unless O_BINARY is different from O_TEXT, first bit-or:ing one
+     * of them in, and then bit-and-masking the other them away, won't
+     * have much of an effect. */
+    switch (*mode) {
+    case 'b':
+#if O_TEXT != O_BINARY
+        oflags |= O_BINARY;
 	oflags &= ~O_TEXT;
-	mode++;
-    }
-    else if (*mode == 't') {
+#endif
+        mode++;
+        break;
+    case 't':
+#if O_TEXT != O_BINARY
 	oflags |= O_TEXT;
 	oflags &= ~O_BINARY;
-	mode++;
-    }
-    else {
+#endif
+        mode++;
+        break;
+    default:
+#  if O_BINARY != 0
+        /* bit-or:ing with zero O_BINARY would be useless. */
 	/*
 	 * If neither "t" nor "b" was specified, open the file
 	 * in O_BINARY mode.
+         *
+         * Note that if something else than the zero byte was seen
+         * here (e.g. bogus mode "rx"), just few lines later we will
+         * set the errno and invalidate the flags.
 	 */
 	oflags |= O_BINARY;
+#  endif
+        break;
     }
-#endif
     if (*mode || oflags == -1) {
 	SETERRNO(EINVAL, LIB_INVARG);
 	oflags = -1;