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")

diff --git a/op.c b/op.c
index 5a7983f..94ff49f 100644 (file)
--- a/op.c
+++ b/op.c
@@ -8623,7 +8623,7 @@ Perl_ck_anoncode(pTHX_ OP *o)
 static void
 S_io_hints(pTHX_ OP *o)
 {
-#ifdef PERLIO_BINARY_AND_TEXT_DIFFERENT_AND_EFFECTIVE
+#if O_BINARY != 0 || O_TEXT != 0
     HV * const table =
        PL_hints & HINT_LOCALIZE_HH ? GvHV(PL_hintgv) : NULL;;
     if (table) {
@@ -8632,10 +8632,15 @@ S_io_hints(pTHX_ OP *o)
            STRLEN len = 0;
            const char *d = SvPV_const(*svp, len);
            const I32 mode = mode_from_discipline(d, len);
+            /* bit-and:ing with zero O_BINARY or O_TEXT would be useless. */
+#  if O_BINARY != 0
            if (mode & O_BINARY)
                o->op_private |= OPpOPEN_IN_RAW;
-           else if (mode & O_TEXT)
+#  endif
+#  if O_TEXT != 0
+           if (mode & O_TEXT)
                o->op_private |= OPpOPEN_IN_CRLF;
+#  endif
        }
 
        svp = hv_fetchs(table, "open_OUT", FALSE);
@@ -8643,10 +8648,15 @@ S_io_hints(pTHX_ OP *o)
            STRLEN len = 0;
            const char *d = SvPV_const(*svp, len);
            const I32 mode = mode_from_discipline(d, len);
+            /* bit-and:ing with zero O_BINARY or O_TEXT would be useless. */
+#  if O_BINARY != 0
            if (mode & O_BINARY)
                o->op_private |= OPpOPEN_OUT_RAW;
-           else if (mode & O_TEXT)
+#  endif
+#  if O_TEXT != 0
+           if (mode & O_TEXT)
                o->op_private |= OPpOPEN_OUT_CRLF;
+#  endif
        }
     }
 #else

diff --git a/perl.h b/perl.h
index acf0ee9..7338f61 100644 (file)
--- a/perl.h
+++ b/perl.h
@@ -5689,14 +5689,6 @@ int flock(int fd, int op);
     /* If you really are DOSish. */
 #      define PERLIO_USING_CRLF 1
 #   endif
-#   if O_TEXT != 0 && O_BINARY != 0
-#     if !defined(__HAIKU__)
-        /* If you have O_TEXT different from your O_BINARY and
-         * they are both not zero (being zero would make testing
-         * with bit-and interesting) and they have an effect. */
-#       define PERLIO_BINARY_AND_TEXT_DIFFERENT_AND_EFFECTIVE
-#     endif
-#   endif
 #endif
 
 #ifdef I_LIBUTIL

diff --git a/perlio.c b/perlio.c
index c4ace68..90622c1 100644 (file)
--- 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;