[PATCH] binary-io: do not treat set_binary_mode() on stdin/out/err as an error on OS/2

[PATCH] binary-io: do not treat set_binary_mode() on stdin/out/err as an error on OS/2

[KO Myung-Hun]

Setting stdin/out/err to binary mode is allowed on OS/2. But it's not
useful, because it generates stair-output hard to read.

Instead, let's set them to text mode all the time.

This fixes that tee always crashes at startup if stdin or stdout
are not piped.

* lib/binary-io.c (__gl_setmode_check) [__EMX__]: Remove __EMX__ guard.
* lib/binary-io.h (__gl_setmode_check) [__EMX__]: Remove __EMX__ guard.
(set_binary_mode) [__EMX__]: Override mode with O_TEXT if tty.
---
 lib/binary-io.c | 2 +-
 lib/binary-io.h | 8 +++++++-
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/lib/binary-io.c b/lib/binary-io.c
index 01e0bf647..03b98681b 100644
--- a/lib/binary-io.c
+++ b/lib/binary-io.c
@@ -19,7 +19,7 @@
 #define BINARY_IO_INLINE _GL_EXTERN_INLINE
 #include "binary-io.h"
 
-#if defined __DJGPP__ || defined __EMX__
+#if defined __DJGPP__
 # include <errno.h>
 # include <unistd.h>
 
diff --git a/lib/binary-io.h b/lib/binary-io.h
index 720b08c75..75881bd1c 100644
--- a/lib/binary-io.h
+++ b/lib/binary-io.h
@@ -53,7 +53,7 @@ __gl_setmode (int fd _GL_UNUSED, int mode _GL_UNUSED)
 }
 #endif
 
-#if defined __DJGPP__ || defined __EMX__
+#if defined __DJGPP__
 extern int __gl_setmode_check (int);
 #else
 BINARY_IO_INLINE int
@@ -70,6 +70,12 @@ BINARY_IO_INLINE int
 set_binary_mode (int fd, int mode)
 {
   int r = __gl_setmode_check (fd);
+#if defined __EMX__
+  /* On OS/2, setting stdin/out/err to binary is not an error, but
+     non-sense.  */
+  if (isatty (fd))
+    mode = O_TEXT;
+#endif
   return r != 0 ? r : __gl_setmode (fd, mode);
 }
 
-- 
2.13.3

Re: [PATCH] binary-io: do not treat set_binary_mode() on stdin/out/err as an error on OS/2

[Paul Eggert]

On 5/24/19 4:31 AM, KO Myung-Hun wrote:
> Setting stdin/out/err to binary mode is allowed on OS/2. But it's not
> useful, because it generates stair-output hard to read.
>
> Instead, let's set them to text mode all the time.

First, that patch doesn't set them to text mode all the time; it does so 
only if isatty returns nonzero.

Second, __gl_setmode_check already has the isatty test, so why do it twice?

Re: [PATCH] binary-io: do not treat set_binary_mode() on stdin/out/err as an error on OS/2

[KO Myung-Hun]

Hi/2.

Paul Eggert wrote:
> On 5/24/19 4:31 AM, KO Myung-Hun wrote:
>> Setting stdin/out/err to binary mode is allowed on OS/2. But it's not
>> useful, because it generates stair-output hard to read.
>>
>> Instead, let's set them to text mode all the time.
> 
> First, that patch doesn't set them to text mode all the time; it does so
> only if isatty returns nonzero.
> 

them = stdin/out/err = tty

Therefore, they are always set to text mode whenever set_binary_mode()
is called.

> Second, __gl_setmode_check already has the isatty test, so why do it twice?
> 

First setmode() on tty on OS/2 is not error. However,
__gl_setmode_check() always return -1 if tty.

Second, I'm not sure it's guaranteed that __gl_setmode_check() is
equivalent to isatty().

-- 
KO Myung-Hun

Using Mozilla SeaMonkey 2.7.2
Under OS/2 Warp 4 for Korean with FixPak #15
In VirtualBox v4.1.32 on Intel Core i7-3615QM 2.30GHz with 8GB RAM

Korean OS/2 User Community : http://www.os2.kr/

Re: [PATCH] binary-io: do not treat set_binary_mode() on stdin/out/err as an error on OS/2

[Bruno Haible]

KO Myung-Hun wrote:
> * lib/binary-io.c (__gl_setmode_check) [__EMX__]: Remove __EMX__ guard.
> * lib/binary-io.h (__gl_setmode_check) [__EMX__]: Remove __EMX__ guard.
> (set_binary_mode) [__EMX__]: Override mode with O_TEXT if tty.

According to the EMX documentation of the setmode function
------------------------------------------------------------------------------
#include <io.h>                                                           [PC]
#include <fcntl.h>

int setmode (int handle, int mode);

    Change the text/binary mode of a file handle.  MODE must be either
    O_BINARY or O_TEXT.

    Note: Use _fsetmode() to change the mode of a stream.

  Return value:

    If there's an error, setmode() returns -1 and sets errno to EBADF
    or EINVAL otherwise setmode() returns the previous mode, that is,
    O_BINARY or O_TEXT.

  See also: _fsetmode(), open()
------------------------------------------------------------------------------

your proposed patch has the following effect:


BEFORE:
                                             sets mode to   return value
set_binary_mode on tty                        ---           -1, errno=EINVAL
set_binary_mode on regular file fails         ---           -1, errno=EBADF or EINVAL
set_binary_mode on regular file succeeds      mode          previous mode

AFTER:

                                             sets mode to   return value
set_binary_mode on tty                        O_TEXT        previous mode
set_binary_mode on regular file fails         ---           -1, errno=EBADF or EINVAL
set_binary_mode on regular file succeeds      mode          previous mode


Changing a function to *silently* do a different thing than what it was
requested to do? This is not good. The previous behaviour, to set an error
indicator, is better.

> Setting stdin/out/err to binary mode is allowed on OS/2. But it's not
> useful, because it generates stair-output hard to read.

On a tty, the existing code will refrain from invoking setmode. So no
stair-output in this case.

On a regular file, it depends on your text viewer. Emacs surely doesn't
produce stair-cased rendering of a file.

> Instead, let's set them to text mode all the time.
> 
> This fixes that tee always crashes at startup if stdin or stdout
> are not piped.

1) I don't see why the proposed patch would fix a crash. It only
   changes the way output is rendered outside of the program.
2) The proper place to do such changes is the 'tee' program.

Bruno

Re: [PATCH] binary-io: do not treat set_binary_mode() on stdin/out/err as an error on OS/2

[KO Myung-Hun]

Hi/2.

Bruno Haible wrote:
> KO Myung-Hun wrote:
>> * lib/binary-io.c (__gl_setmode_check) [__EMX__]: Remove __EMX__ guard.
>> * lib/binary-io.h (__gl_setmode_check) [__EMX__]: Remove __EMX__ guard.
>> (set_binary_mode) [__EMX__]: Override mode with O_TEXT if tty.
> 
> According to the EMX documentation of the setmode function
> ------------------------------------------------------------------------------
> #include <io.h>                                                           [PC]
> #include <fcntl.h>
> 
> int setmode (int handle, int mode);
> 
>     Change the text/binary mode of a file handle.  MODE must be either
>     O_BINARY or O_TEXT.
> 
>     Note: Use _fsetmode() to change the mode of a stream.
> 
>   Return value:
> 
>     If there's an error, setmode() returns -1 and sets errno to EBADF
>     or EINVAL otherwise setmode() returns the previous mode, that is,
>     O_BINARY or O_TEXT.
> 
>   See also: _fsetmode(), open()
> ------------------------------------------------------------------------------
> 
> your proposed patch has the following effect:
> 
> 
> BEFORE:
>                                              sets mode to   return value
> set_binary_mode on tty                        ---           -1, errno=EINVAL
> set_binary_mode on regular file fails         ---           -1, errno=EBADF or EINVAL
> set_binary_mode on regular file succeeds      mode          previous mode
> 
> AFTER:
> 
>                                              sets mode to   return value
> set_binary_mode on tty                        O_TEXT        previous mode
> set_binary_mode on regular file fails         ---           -1, errno=EBADF or EINVAL
> set_binary_mode on regular file succeeds      mode          previous mode
> 
> 
> Changing a function to *silently* do a different thing than what it was
> requested to do? This is not good. The previous behaviour, to set an error
> indicator, is better.
> 

I agree. But previous set_binary_mode() also alter the behavior of
setmode() on OS/2, because setmode(O_BINARY) on tty works well instead
of returning -1 as previous set_binary_mode() does.

In addition, set_binary_mode() breaks the compatibility with
SET_BINARY() before set_binary_mode(). Because SET_BINARY() does nothing
on tty. It does not cause any error at all. My patch is compatible with
SET_BINARY().

>> Setting stdin/out/err to binary mode is allowed on OS/2. But it's not
>> useful, because it generates stair-output hard to read.
> 
> On a tty, the existing code will refrain from invoking setmode. So no
> stair-output in this case.
> 

Instead, it return an error. If some codes checks the error code, it
will determine that an error occurs.

Especially, programs using xset_binary_mode() will abort due to that
error. tee of coreutils is a good example.

> On a regular file, it depends on your text viewer. Emacs surely doesn't
> produce stair-cased rendering of a file.
> 
>> Instead, let's set them to text mode all the time.
>>
>> This fixes that tee always crashes at startup if stdin or stdout
>> are not piped.
> 
> 1) I don't see why the proposed patch would fix a crash. It only
>    changes the way output is rendered outside of the program.

Sorry, 'crash' is not proper word. Maybe 'abort' is more proper. Anyway,
the 'abort' is the result of xset_binary_mode() using the result of
set_binary_mode() which affected by __gl_setmode_check().

> 2) The proper place to do such changes is the 'tee' program.
> 

No.many programs of coreutils including tee are using
xset_binary_mode(). Maybe more programs using xset_binary_mode() of
gnulib. And they will abort as soon as trying to use xset_binary_mode()
at startup.


Therefore, the proper place is gnulib.

-- 
KO Myung-Hun

Using Mozilla SeaMonkey 2.7.2
Under OS/2 Warp 4 for Korean with FixPak #15
In VirtualBox v6.0.8 on Intel Core i7-3615QM 2.30GHz with 8GB RAM

Korean OS/2 User Community : http://www.os2.kr/

Re: [PATCH] binary-io: do not treat set_binary_mode() on stdin/out/err as an error on OS/2

[Bruno Haible]

KO Myung-Hun wrote:
> setmode(O_BINARY) on tty works well instead
> of returning -1 as previous set_binary_mode() does.

"works well", but isn't this the case which produces staircase-shaped
output?

> In addition, set_binary_mode() breaks the compatibility with
> SET_BINARY() before set_binary_mode(). Because SET_BINARY() does nothing
> on tty. It does not cause any error at all. My patch is compatible with
> SET_BINARY().

So you have a problem with the patches by Paul in
<https://lists.gnu.org/archive/html/bug-gnulib/2017-02/msg00058.html>
<https://lists.gnu.org/archive/html/bug-gnulib/2017-02/msg00062.html>
and then in coreutils 
https://git.savannah.gnu.org/gitweb/?p=coreutils.git;a=commitdiff;h=75aababed45d0120d44baa76c5107d0ceb71fc59
https://git.savannah.gnu.org/gitweb/?p=coreutils.git;a=commitdiff;h=1c27b56095b4a82be7d072baabc09262cb4227e5

Paul, what was the problem that motivated these patches?
Was it <https://lists.gnu.org/archive/html/bug-gnulib/2009-11/msg00293.html> ?

There are comments, for example in tr.c, that explain why these programs
want to avoid conversion between LF and CR/LF:

  /* Use binary I/O, since 'tr' is sometimes used to transliterate
     non-printable characters, or characters which are stripped away
     by text-mode reads (like CR and ^Z).  */
  xset_binary_mode (STDIN_FILENO, O_BINARY);
  xset_binary_mode (STDOUT_FILENO, O_BINARY);

Error checking has been added because we want reliable software.

> the 'abort' is the result of xset_binary_mode() using the result of
> set_binary_mode() which affected by __gl_setmode_check().
> 
> No.many programs of coreutils including tee are using
> xset_binary_mode(). Maybe more programs using xset_binary_mode() of
> gnulib. And they will abort as soon as trying to use xset_binary_mode()
> at startup.

OK, then maybe the fix is in gnulib. Your justification sounded like only
the 'tee' program was affected.

You will need to argue what is the desired behaviour, before proposing
a patch.

* What is the default mode of stdin/out/err when it is not a tty?

* Is this default desirable? If not, why not?

* What is the default mode of stdin/out/err when it is a tty?

* Is this default desirable? If not, why not?

* When does staircase-shaped output occur?

* What is the result of isatty(1) 
  a) for console output?
  b) for pipe output?
  c) for regular file output?

* What is the result of isatty(0) 
  a) for console input?
  b) for pipe input?
  c) for regular file input?

* Can setmode fail, other than when the fd argument is invalid (-> EBADF) or
  the mode argument is invalid (-> EINVAL)?

When you have provided the answers to these questions, maybe we'll get closer.

Bruno

Re: [PATCH] binary-io: do not treat set_binary_mode() on stdin/out/err as an error on OS/2

[Paul Eggert]

Bruno Haible wrote:
> So you have a problem with the patches by Paul in
> <https://lists.gnu.org/archive/html/bug-gnulib/2017-02/msg00058.html>
> <https://lists.gnu.org/archive/html/bug-gnulib/2017-02/msg00062.html>
> and then in coreutils
> https://git.savannah.gnu.org/gitweb/?p=coreutils.git;a=commitdiff;h=75aababed45d0120d44baa76c5107d0ceb71fc59
> https://git.savannah.gnu.org/gitweb/?p=coreutils.git;a=commitdiff;h=1c27b56095b4a82be7d072baabc09262cb4227e5
> 
> Paul, what was the problem that motivated these patches?
> Was it<https://lists.gnu.org/archive/html/bug-gnulib/2009-11/msg00293.html>  ?

Yes, I believe that's right. Thanks for looking into this.

Re: [PATCH] binary-io: do not treat set_binary_mode() on stdin/out/err as an error on OS/2

[KO Myung-Hun]

Bruno Haible wrote:
> KO Myung-Hun wrote:
>> setmode(O_BINARY) on tty works well instead
>> of returning -1 as previous set_binary_mode() does.
> 
> "works well", but isn't this the case which produces staircase-shaped
> output?
> 

setmode(O_BINARY) on tty causes the staircase-shaped output. And read
CRLF pair not LF from input device.

>> In addition, set_binary_mode() breaks the compatibility with
>> SET_BINARY() before set_binary_mode(). Because SET_BINARY() does nothing
>> on tty. It does not cause any error at all. My patch is compatible with
>> SET_BINARY().
> 
> So you have a problem with the patches by Paul in
> <https://lists.gnu.org/archive/html/bug-gnulib/2017-02/msg00058.html>
> <https://lists.gnu.org/archive/html/bug-gnulib/2017-02/msg00062.html>
> and then in coreutils 
> https://git.savannah.gnu.org/gitweb/?p=coreutils.git;a=commitdiff;h=75aababed45d0120d44baa76c5107d0ceb71fc59
> https://git.savannah.gnu.org/gitweb/?p=coreutils.git;a=commitdiff;h=1c27b56095b4a82be7d072baabc09262cb4227e5
> 
> Paul, what was the problem that motivated these patches?
> Was it <https://lists.gnu.org/archive/html/bug-gnulib/2009-11/msg00293.html> ?
> 
> There are comments, for example in tr.c, that explain why these programs
> want to avoid conversion between LF and CR/LF:
> 
>   /* Use binary I/O, since 'tr' is sometimes used to transliterate
>      non-printable characters, or characters which are stripped away
>      by text-mode reads (like CR and ^Z).  */
>   xset_binary_mode (STDIN_FILENO, O_BINARY);
>   xset_binary_mode (STDOUT_FILENO, O_BINARY);
> 
> Error checking has been added because we want reliable software.
> 
>> the 'abort' is the result of xset_binary_mode() using the result of
>> set_binary_mode() which affected by __gl_setmode_check().
>>
>> No.many programs of coreutils including tee are using
>> xset_binary_mode(). Maybe more programs using xset_binary_mode() of
>> gnulib. And they will abort as soon as trying to use xset_binary_mode()
>> at startup.
> 
> OK, then maybe the fix is in gnulib. Your justification sounded like only
> the 'tee' program was affected.
> 
> You will need to argue what is the desired behaviour, before proposing
> a patch.
> 

No problem.

> * What is the default mode of stdin/out/err when it is not a tty?
> 

Any files and pipes are opened in text mode by default if a user provide
neither a specific mode nor a specific setting.

As a result, the default mode of stdin/out/err if not a tty is text mode.

However, if stdin/out/err is the non-tty whose mode is binary mode then
its mode becomes binary mode.


> * Is this default desirable? If not, why not?
> 

Of course. It's default behavior on OS/2. All the OS/2 programs and
programmers expect this default.

> * What is the default mode of stdin/out/err when it is a tty?
> 

Text mode.

> * Is this default desirable? If not, why not?
>

Desirable. Tty on OS/2 work in text mode.

> * When does staircase-shaped output occur?
> 

When stdout which is a tty is in binary mode.
So does stderr.

> * What is the result of isatty(1) 
>   a) for console output?
1

>   b) for pipe output?
0

>   c) for regular file output?
0

> 
> * What is the result of isatty(0) 
>   a) for console input?
1

>   b) for pipe input?
0

>   c) for regular file input?
0

> 
> * Can setmode fail, other than when the fd argument is invalid (-> EBADF) or
>   the mode argument is invalid (-> EINVAL)?
> 
> When you have provided the answers to these questions, maybe we'll get closer.
> 

I hope this help.


-- 
KO Myung-Hun

Using Mozilla SeaMonkey 2.7.2
Under OS/2 Warp 4 for Korean with FixPak #15
In VirtualBox v6.0.8 on Intel Core i7-3615QM 2.30GHz with 8GB RAM

Korean OS/2 User Community : http://www.os2.kr/

Re: [PATCH] binary-io: do not treat set_binary_mode() on stdin/out/err as an error on OS/2

[Bruno Haible]

Hi KO,

Thanks for the answers.

> > * What is the result of isatty(1) 
> >   a) for console output?
> 1
> 
> >   b) for pipe output?
> 0
> 
> >   c) for regular file output?
> 0
> 
> > 
> > * What is the result of isatty(0) 
> >   a) for console input?
> 1
> 
> >   b) for pipe input?
> 0
> 
> >   c) for regular file input?
> 0

OK, so isatty() is the right test.

> > * What is the default mode of stdin/out/err when it is not a tty?
> > 
> 
> Any files and pipes are opened in text mode by default if a user provide
> neither a specific mode nor a specific setting.
> 
> As a result, the default mode of stdin/out/err if not a tty is text mode.

OK, so tee.c, tr.c etc. ought to contain the equivalent of:

  if (! isatty (STDIN_FILENO))
    setmode (STDIN_FILENO, O_BINARY);
  if (! isatty (STDOUT_FILENO))
    setmode (STDOUT_FILENO, O_BINARY);

> > * What is the default mode of stdin/out/err when it is a tty?
> > 
> 
> Text mode.
> 
> > * When does staircase-shaped output occur?
> > 
> 
> When stdout which is a tty is in binary mode.
> So does stderr.

So when isatty (fd), we DON'T want setmode (fd, O_BINARY).

In other words, the desired behaviour is:

                                             sets mode to   return value
set_binary_mode on tty                        ---           0 or previous mode
set_binary_mode on regular file fails         ---           -1, errno=EBADF or EINVAL
set_binary_mode on regular file succeeds      mode          previous mode


Here is a proposed patch. It should have the same effects as your original
patch, but with some comment and without turning O_BINARY into its opposite.
And extended to DJGPP, since DJGPP surely has the same problems as EMX. I
don't know why nobody has reported the problem on DJGPP in two years.

It reverts parts of Paul's patch
<https://lists.gnu.org/archive/html/bug-gnulib/2017-02/msg00058.html>, namely:
* set_binary_mode on a tty was changed in 2017 to return an error code and
  now again succeeds.
* Inline and remove the __gl_setmode_check function. I found that the
  indirection through this function makes the code harder to understand.


2019-05-28  Bruno Haible  <bruno@clisp.org>

	binary-io: Attempted use of O_BINARY on consoles no longer fails.
	Reported by KO Myung-Hun <komh78@gmail.com> in
	<https://lists.gnu.org/archive/html/bug-gnulib/2019-05/msg00124.html>.
	* lib/binary-io.h (__gl_setmode_check): Remove function.
	(set_binary_mode): Declare as notinline on DJGPP and EMX.
	* lib/binary-io.c (__gl_setmode_check): Remove function.
	(set_binary_mode): Define here on DJGPP and EMX. Inline
	__gl_setmode_check. In case of a tty, don't return an error code.

diff --git a/lib/binary-io.h b/lib/binary-io.h
index 720b08c..8d4133b 100644
--- a/lib/binary-io.h
+++ b/lib/binary-io.h
@@ -53,25 +53,21 @@ __gl_setmode (int fd _GL_UNUSED, int mode _GL_UNUSED)
 }
 #endif
 
-#if defined __DJGPP__ || defined __EMX__
-extern int __gl_setmode_check (int);
-#else
-BINARY_IO_INLINE int
-__gl_setmode_check (int fd _GL_UNUSED) { return 0; }
-#endif
-
 /* Set FD's mode to MODE, which should be either O_TEXT or O_BINARY.
    Return the old mode if successful, -1 (setting errno) on failure.
    Ordinarily this function would be called 'setmode', since that is
    its name on MS-Windows, but it is called 'set_binary_mode' here
    to avoid colliding with a BSD function of another name.  */
 
+#if defined __DJGPP__ || defined __EMX__
+extern int set_binary_mode (int fd, int mode);
+#else
 BINARY_IO_INLINE int
 set_binary_mode (int fd, int mode)
 {
-  int r = __gl_setmode_check (fd);
-  return r != 0 ? r : __gl_setmode (fd, mode);
+  return __gl_setmode (fd, mode);
 }
+#endif
 
 /* This macro is obsolescent.  */
 #define SET_BINARY(fd) ((void) set_binary_mode (fd, O_BINARY))
diff --git a/lib/binary-io.c b/lib/binary-io.c
index 01e0bf6..77490e6 100644
--- a/lib/binary-io.c
+++ b/lib/binary-io.c
@@ -20,18 +20,20 @@
 #include "binary-io.h"
 
 #if defined __DJGPP__ || defined __EMX__
-# include <errno.h>
 # include <unistd.h>
 
 int
-__gl_setmode_check (int fd)
+set_binary_mode (int fd, int mode)
 {
   if (isatty (fd))
-    {
-      errno = EINVAL;
-      return -1;
-    }
+    /* If FD refers to a console (not a pipe, not a regular file),
+       O_TEXT is the only reasonable mode, both on input and on output.
+       Silently ignore the request.  If we were to return -1 here,
+       all programs that use xset_binary_mode would fail when run
+       with console input or console output.  */
+    return O_TEXT;
   else
-    return 0;
+    return __gl_setmode (fd, mode);
 }
+
 #endif

Re: [PATCH] binary-io: do not treat set_binary_mode() on stdin/out/err as an error on OS/2

[KO Myung-Hun]

Hi/2.

It works fine here as expected.

Thanks!

Bruno Haible wrote:
> Hi KO,
> 
> Thanks for the answers.
> 
>>> * What is the result of isatty(1) 
>>>   a) for console output?
>> 1
>>
>>>   b) for pipe output?
>> 0
>>
>>>   c) for regular file output?
>> 0
>>
>>>
>>> * What is the result of isatty(0) 
>>>   a) for console input?
>> 1
>>
>>>   b) for pipe input?
>> 0
>>
>>>   c) for regular file input?
>> 0
> 
> OK, so isatty() is the right test.
> 
>>> * What is the default mode of stdin/out/err when it is not a tty?
>>>
>>
>> Any files and pipes are opened in text mode by default if a user provide
>> neither a specific mode nor a specific setting.
>>
>> As a result, the default mode of stdin/out/err if not a tty is text mode.
> 
> OK, so tee.c, tr.c etc. ought to contain the equivalent of:
> 
>   if (! isatty (STDIN_FILENO))
>     setmode (STDIN_FILENO, O_BINARY);
>   if (! isatty (STDOUT_FILENO))
>     setmode (STDOUT_FILENO, O_BINARY);
> 
>>> * What is the default mode of stdin/out/err when it is a tty?
>>>
>>
>> Text mode.
>>
>>> * When does staircase-shaped output occur?
>>>
>>
>> When stdout which is a tty is in binary mode.
>> So does stderr.
> 
> So when isatty (fd), we DON'T want setmode (fd, O_BINARY).
> 
> In other words, the desired behaviour is:
> 
>                                              sets mode to   return value
> set_binary_mode on tty                        ---           0 or previous mode
> set_binary_mode on regular file fails         ---           -1, errno=EBADF or EINVAL
> set_binary_mode on regular file succeeds      mode          previous mode
> 
> 
> Here is a proposed patch. It should have the same effects as your original
> patch, but with some comment and without turning O_BINARY into its opposite.
> And extended to DJGPP, since DJGPP surely has the same problems as EMX. I
> don't know why nobody has reported the problem on DJGPP in two years.
> 
> It reverts parts of Paul's patch
> <https://lists.gnu.org/archive/html/bug-gnulib/2017-02/msg00058.html>, namely:
> * set_binary_mode on a tty was changed in 2017 to return an error code and
>   now again succeeds.
> * Inline and remove the __gl_setmode_check function. I found that the
>   indirection through this function makes the code harder to understand.
> 
> 
> 2019-05-28  Bruno Haible  <bruno@clisp.org>
> 
> 	binary-io: Attempted use of O_BINARY on consoles no longer fails.
> 	Reported by KO Myung-Hun <komh78@gmail.com> in
> 	<https://lists.gnu.org/archive/html/bug-gnulib/2019-05/msg00124.html>.
> 	* lib/binary-io.h (__gl_setmode_check): Remove function.
> 	(set_binary_mode): Declare as notinline on DJGPP and EMX.
> 	* lib/binary-io.c (__gl_setmode_check): Remove function.
> 	(set_binary_mode): Define here on DJGPP and EMX. Inline
> 	__gl_setmode_check. In case of a tty, don't return an error code.
> 
> diff --git a/lib/binary-io.h b/lib/binary-io.h
> index 720b08c..8d4133b 100644
> --- a/lib/binary-io.h
> +++ b/lib/binary-io.h
> @@ -53,25 +53,21 @@ __gl_setmode (int fd _GL_UNUSED, int mode _GL_UNUSED)
>  }
>  #endif
>  
> -#if defined __DJGPP__ || defined __EMX__
> -extern int __gl_setmode_check (int);
> -#else
> -BINARY_IO_INLINE int
> -__gl_setmode_check (int fd _GL_UNUSED) { return 0; }
> -#endif
> -
>  /* Set FD's mode to MODE, which should be either O_TEXT or O_BINARY.
>     Return the old mode if successful, -1 (setting errno) on failure.
>     Ordinarily this function would be called 'setmode', since that is
>     its name on MS-Windows, but it is called 'set_binary_mode' here
>     to avoid colliding with a BSD function of another name.  */
>  
> +#if defined __DJGPP__ || defined __EMX__
> +extern int set_binary_mode (int fd, int mode);
> +#else
>  BINARY_IO_INLINE int
>  set_binary_mode (int fd, int mode)
>  {
> -  int r = __gl_setmode_check (fd);
> -  return r != 0 ? r : __gl_setmode (fd, mode);
> +  return __gl_setmode (fd, mode);
>  }
> +#endif
>  
>  /* This macro is obsolescent.  */
>  #define SET_BINARY(fd) ((void) set_binary_mode (fd, O_BINARY))
> diff --git a/lib/binary-io.c b/lib/binary-io.c
> index 01e0bf6..77490e6 100644
> --- a/lib/binary-io.c
> +++ b/lib/binary-io.c
> @@ -20,18 +20,20 @@
>  #include "binary-io.h"
>  
>  #if defined __DJGPP__ || defined __EMX__
> -# include <errno.h>
>  # include <unistd.h>
>  
>  int
> -__gl_setmode_check (int fd)
> +set_binary_mode (int fd, int mode)
>  {
>    if (isatty (fd))
> -    {
> -      errno = EINVAL;
> -      return -1;
> -    }
> +    /* If FD refers to a console (not a pipe, not a regular file),
> +       O_TEXT is the only reasonable mode, both on input and on output.
> +       Silently ignore the request.  If we were to return -1 here,
> +       all programs that use xset_binary_mode would fail when run
> +       with console input or console output.  */
> +    return O_TEXT;
>    else
> -    return 0;
> +    return __gl_setmode (fd, mode);
>  }
> +
>  #endif
> 

-- 
KO Myung-Hun

Using Mozilla SeaMonkey 2.7.2
Under OS/2 Warp 4 for Korean with FixPak #15
In VirtualBox v6.0.8 on Intel Core i7-3615QM 2.30GHz with 8GB RAM

Korean OS/2 User Community : http://www.os2.kr/

Re: [PATCH] binary-io: do not treat set_binary_mode() on stdin/out/err as an error on OS/2

[Paul Eggert]

On 5/28/19 5:47 PM, Bruno Haible wrote:
> OK, so tee.c, tr.c etc. ought to contain the equivalent of:
>
>    if (! isatty (STDIN_FILENO))
>      setmode (STDIN_FILENO, O_BINARY);
>    if (! isatty (STDOUT_FILENO))
>      setmode (STDOUT_FILENO, O_BINARY);

That sounds overly complicated. Can't we arrange to have something 
simpler, like what we have now?

   xset_binary_mode (STDIN_FILENO, O_BINARY);
   xset_binary_mode (STDOUT_FILENO, O_BINARY);

The idea is that tee.c, tr.c, etc. should not have to worry about 
calling isatty.

Re: [PATCH] binary-io: do not treat set_binary_mode() on stdin/out/err as an error on OS/2

[Bruno Haible]

Hi Paul,

> > OK, so tee.c, tr.c etc. ought to contain the equivalent of:
> >
> >    if (! isatty (STDIN_FILENO))
> >      setmode (STDIN_FILENO, O_BINARY);
> >    if (! isatty (STDOUT_FILENO))
> >      setmode (STDOUT_FILENO, O_BINARY);

This code snippet is what we would have to write in every program,
IF THERE WAS NO GNULIB.

> That sounds overly complicated. Can't we arrange to have something 
> simpler, like what we have now?
> 
>    xset_binary_mode (STDIN_FILENO, O_BINARY);
>    xset_binary_mode (STDOUT_FILENO, O_BINARY);
> 
> The idea is that tee.c, tr.c, etc. should not have to worry about 
> calling isatty.

Yes! This is already how the coreutils code looks like, and what I propose
is a patch to gnulib ONLY [1]. It does NOT require changes to coreutils.

Bruno

[1] the second half of
    https://lists.gnu.org/archive/html/bug-gnulib/2019-05/msg00168.html

Re: [PATCH] binary-io: do not treat set_binary_mode() on stdin/out/err as an error on OS/2

[Paul Eggert]

Bruno Haible wrote:
> Yes! This is already how the coreutils code looks like, and what I propose
> is a patch to gnulib ONLY [1]. It does NOT require changes to coreutils.
> 
> Bruno
> 
> [1] the second half of
>      https://lists.gnu.org/archive/html/bug-gnulib/2019-05/msg00168.html

Thanks, that sounds good then.

Re: [PATCH] binary-io: do not treat set_binary_mode() on stdin/out/err as an error on OS/2

[Bruno Haible]

Paul Eggert wrote:
> Thanks, that sounds good then.

OK, I push it.

Bruno