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  <address@hidden>
> 
>       binary-io: Attempted use of O_BINARY on consoles no longer fails.
>       Reported by KO Myung-Hun <address@hidden> 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/