bug-gnulib@gnu.org mirror (unofficial)
 help / color / mirror / Atom feed
From: Bruno Haible <bruno@clisp.org>
To: KO Myung-Hun <komh78@gmail.com>
Cc: bug-gnulib@gnu.org
Subject: Re: [PATCH] binary-io: do not treat set_binary_mode() on stdin/out/err as an error on OS/2
Date: Wed, 29 May 2019 02:47:31 +0200	[thread overview]
Message-ID: <2890383.TVOU5uu8mX@omega> (raw)
In-Reply-To: <5CE9F088.4060504@gmail.com>

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



  reply	other threads:[~2019-05-29  0:47 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-24 11:31 [PATCH] binary-io: do not treat set_binary_mode() on stdin/out/err as an error on OS/2 KO Myung-Hun
2019-05-24 20:20 ` Paul Eggert
2019-05-25  3:29   ` KO Myung-Hun
2019-05-25 13:41 ` Bruno Haible
2019-05-25 14:18   ` KO Myung-Hun
2019-05-25 16:31     ` Bruno Haible
2019-05-25 17:14       ` Paul Eggert
2019-05-26  1:48       ` KO Myung-Hun
2019-05-29  0:47         ` Bruno Haible [this message]
2019-05-29 13:32           ` KO Myung-Hun
2019-05-30  0:05           ` Paul Eggert
2019-05-30  0:55             ` Bruno Haible
2019-06-02  6:03               ` Paul Eggert
2019-06-02  9:31                 ` Bruno Haible

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://lists.gnu.org/mailman/listinfo/bug-gnulib

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2890383.TVOU5uu8mX@omega \
    --to=bruno@clisp.org \
    --cc=bug-gnulib@gnu.org \
    --cc=komh78@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).