* [PATCH] binary-io: do not treat set_binary_mode() on stdin/out/err as an error on OS/2 @ 2019-05-24 11:31 KO Myung-Hun 2019-05-24 20:20 ` Paul Eggert 2019-05-25 13:41 ` Bruno Haible 0 siblings, 2 replies; 14+ messages in thread From: KO Myung-Hun @ 2019-05-24 11:31 UTC (permalink / raw) To: bug-gnulib 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 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH] binary-io: do not treat set_binary_mode() on stdin/out/err as an error on OS/2 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 1 sibling, 1 reply; 14+ messages in thread From: Paul Eggert @ 2019-05-24 20:20 UTC (permalink / raw) To: KO Myung-Hun; +Cc: bug-gnulib 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? ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] binary-io: do not treat set_binary_mode() on stdin/out/err as an error on OS/2 2019-05-24 20:20 ` Paul Eggert @ 2019-05-25 3:29 ` KO Myung-Hun 0 siblings, 0 replies; 14+ messages in thread From: KO Myung-Hun @ 2019-05-25 3:29 UTC (permalink / raw) To: Paul Eggert; +Cc: bug-gnulib 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/ ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] binary-io: do not treat set_binary_mode() on stdin/out/err as an error on OS/2 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 13:41 ` Bruno Haible 2019-05-25 14:18 ` KO Myung-Hun 1 sibling, 1 reply; 14+ messages in thread From: Bruno Haible @ 2019-05-25 13:41 UTC (permalink / raw) To: bug-gnulib; +Cc: KO Myung-Hun 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] binary-io: do not treat set_binary_mode() on stdin/out/err as an error on OS/2 2019-05-25 13:41 ` Bruno Haible @ 2019-05-25 14:18 ` KO Myung-Hun 2019-05-25 16:31 ` Bruno Haible 0 siblings, 1 reply; 14+ messages in thread From: KO Myung-Hun @ 2019-05-25 14:18 UTC (permalink / raw) To: Bruno Haible; +Cc: bug-gnulib 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/ ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] binary-io: do not treat set_binary_mode() on stdin/out/err as an error on OS/2 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 0 siblings, 2 replies; 14+ messages in thread From: Bruno Haible @ 2019-05-25 16:31 UTC (permalink / raw) To: KO Myung-Hun; +Cc: bug-gnulib 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] binary-io: do not treat set_binary_mode() on stdin/out/err as an error on OS/2 2019-05-25 16:31 ` Bruno Haible @ 2019-05-25 17:14 ` Paul Eggert 2019-05-26 1:48 ` KO Myung-Hun 1 sibling, 0 replies; 14+ messages in thread From: Paul Eggert @ 2019-05-25 17:14 UTC (permalink / raw) To: Bruno Haible, KO Myung-Hun; +Cc: bug-gnulib 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. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] binary-io: do not treat set_binary_mode() on stdin/out/err as an error on OS/2 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 1 sibling, 1 reply; 14+ messages in thread From: KO Myung-Hun @ 2019-05-26 1:48 UTC (permalink / raw) To: Bruno Haible; +Cc: bug-gnulib 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/ ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] binary-io: do not treat set_binary_mode() on stdin/out/err as an error on OS/2 2019-05-26 1:48 ` KO Myung-Hun @ 2019-05-29 0:47 ` Bruno Haible 2019-05-29 13:32 ` KO Myung-Hun 2019-05-30 0:05 ` Paul Eggert 0 siblings, 2 replies; 14+ messages in thread From: Bruno Haible @ 2019-05-29 0:47 UTC (permalink / raw) To: KO Myung-Hun; +Cc: bug-gnulib 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 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH] binary-io: do not treat set_binary_mode() on stdin/out/err as an error on OS/2 2019-05-29 0:47 ` Bruno Haible @ 2019-05-29 13:32 ` KO Myung-Hun 2019-05-30 0:05 ` Paul Eggert 1 sibling, 0 replies; 14+ messages in thread From: KO Myung-Hun @ 2019-05-29 13:32 UTC (permalink / raw) To: Bruno Haible; +Cc: bug-gnulib 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/ ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] binary-io: do not treat set_binary_mode() on stdin/out/err as an error on OS/2 2019-05-29 0:47 ` Bruno Haible 2019-05-29 13:32 ` KO Myung-Hun @ 2019-05-30 0:05 ` Paul Eggert 2019-05-30 0:55 ` Bruno Haible 1 sibling, 1 reply; 14+ messages in thread From: Paul Eggert @ 2019-05-30 0:05 UTC (permalink / raw) To: Bruno Haible; +Cc: bug-gnulib, KO Myung-Hun 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. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] binary-io: do not treat set_binary_mode() on stdin/out/err as an error on OS/2 2019-05-30 0:05 ` Paul Eggert @ 2019-05-30 0:55 ` Bruno Haible 2019-06-02 6:03 ` Paul Eggert 0 siblings, 1 reply; 14+ messages in thread From: Bruno Haible @ 2019-05-30 0:55 UTC (permalink / raw) To: Paul Eggert; +Cc: bug-gnulib, KO Myung-Hun 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] binary-io: do not treat set_binary_mode() on stdin/out/err as an error on OS/2 2019-05-30 0:55 ` Bruno Haible @ 2019-06-02 6:03 ` Paul Eggert 2019-06-02 9:31 ` Bruno Haible 0 siblings, 1 reply; 14+ messages in thread From: Paul Eggert @ 2019-06-02 6:03 UTC (permalink / raw) To: Bruno Haible; +Cc: bug-gnulib, KO Myung-Hun 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. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] binary-io: do not treat set_binary_mode() on stdin/out/err as an error on OS/2 2019-06-02 6:03 ` Paul Eggert @ 2019-06-02 9:31 ` Bruno Haible 0 siblings, 0 replies; 14+ messages in thread From: Bruno Haible @ 2019-06-02 9:31 UTC (permalink / raw) To: Paul Eggert; +Cc: bug-gnulib, KO Myung-Hun Paul Eggert wrote: > Thanks, that sounds good then. OK, I push it. Bruno ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2019-06-02 9:31 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 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 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
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).