GNU bug report logs - #18006
Simplify via set_binary_mode

Previous Next

Full log

Message #8 received at 18006 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: 18006 <at> debbugs.gnu.org
Subject: Re: bug#18006: Simplify via set_binary_mode
Date: Sun, 13 Jul 2014 07:20:26 +0300

> Date: Sat, 12 Jul 2014 13:52:28 -0700
> From: Paul Eggert <eggert <at> cs.ucla.edu>
> 
> Attached is a proposed minor simplification to have Emacs use 
> set_binary_mode more consistently.  I'm filing this as a bug report to 
> give Eli a heads-up, as it affects the Microsoft ports.  This is 
> relative to trunk bzr 117525.

Thanks.

However, I don't understand what is the purpose of this patch.  If we
want to clean up uses of setmode and related issues, I'm all for it
(some of that code is old and unnecessary).  But then the Gnulib
binary-io module is not the answer, or at least not all of it.  Some
of the reasons:

  . in some places, we want all file I/O to be in binary mode; Gnulib
    doesn't support that

  . some of the code needs to switch a file descriptor to binary or
    text mode only when the descriptor is or isn't connected to a
    terminal device; Gnulib is ambivalent about that (it always does
    that for MSDOS, and never for Windows)

  . AFAIK, Gnulib's binary-io also replaces isatty, which is not
    really needed in Emacs (you don't show the patches for lib/ so I
    can only guess)

  . we don't want in Emacs the msvc-nothrow wrappers for library
    functions

Some specific comments about the patch:

> @@ -155,20 +148,12 @@
>  
>        if (un_flag)
>  	{
> -	  char buf[18];
> +	  set_binary_mode (fileno (stdout), O_BINARY);
>  
> -#ifdef DOS_NT
> -#if (__DJGPP__ >= 2) || (defined WINDOWSNT)
> -          if (!isatty (fileno (stdout)))
> -	    setmode (fileno (stdout), O_BINARY);
> -#else
> -	  (stdout)->_flag &= ~_IOTEXT; /* print binary */
> -	  _setmode (fileno (stdout), O_BINARY);
> -#endif
> -#endif

We no longer support DJGPP < 2, so the #else stuff could simply go
away.

> -#ifdef DOS_NT
> -#if (__DJGPP__ >= 2) || (defined WINDOWSNT)
> -          if (!isatty (fileno (fp)))
> -	    setmode (fileno (fp), O_BINARY);
> -#else
> -	  (fp)->_flag &= ~_IOTEXT; /* read binary */
> -	  _setmode (fileno (fp), O_BINARY);
> -#endif
> -#endif

Likewise.

>    /* Don't put CRs in the DOC file.  */
> -#ifdef MSDOS
> -  _fmode = O_BINARY;
> -#if 0  /* Suspicion is that this causes hanging.
> -	  So instead we require people to use -o on MSDOS.  */
> -  (stdout)->_flag &= ~_IOTEXT;
> -  _setmode (fileno (stdout), O_BINARY);
> -#endif
> -  outfile = 0;
> -#endif /* MSDOS */
> -#ifdef WINDOWSNT
> -  _fmode = O_BINARY;
> -  _setmode (fileno (stdout), O_BINARY);
> -#endif /* WINDOWSNT */
> +  set_binary_mode (fileno (stdout), O_BINARY);

This is wrong: setting _fmode affects all I/O, input and output, not
just stdout.  Gnulib's binary-io doesn't have the equivalent
functionality.

> @@ -239,11 +237,8 @@
>    /* Manipulate tty.  */
>    if (hide_char)
>      {
> -      emacs_get_tty (fileno (stdin), &etty);
> -#ifdef WINDOWSNT
> -      if (isatty (fileno (stdin)))
> -	_setmode (fileno (stdin), O_BINARY);
> -#endif
> +      if (emacs_get_tty (fileno (stdin), &etty) == 0)
> +	set_binary_mode (fileno (stdin), O_BINARY);
>        suppress_echo_on_tty (fileno (stdin));

This logic is flawed: if emacs_get_tty failed, then emacs_set_tty
should not be called as well.

>  #endif	/* WINDOWSNT */
> +  return isatty (fd) - 1;

This will not work with Windows isatty, because it doesn't return 1
when fd is connected to a console.

To summarize, if we want to clean up that code, I'm willing to do the
job.  Bug Gnulib's binary-io is not the answer.

Thanks.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.