bug#18006: Simplify via set_binary_mode

[Eli Zaretskii]

> Date: Sat, 12 Jul 2014 22:28:56 -0700
> From: Paul Eggert <eggert@cs.ucla.edu>
> CC: 18006@debbugs.gnu.org
> 
> >    . in some places, we want all file I/O to be in binary mode; Gnulib
> >      doesn't support that
> 
> The Emacs code can continue to use _fmode as before; that part won't be 
> simplified, but one step at a time.

See below.

> >    . 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)
> 
> binary-io has two interfaces; one (set_binary_mode) always does it, on 
> all platforms; the other (SET_BINARY) deliberately has no effect on 
> __DJGPP__ when isatty returns nonzero.  Emacs can use either interface, 
> as it needs.

OK.

> Should SET_BINARY also should have no effect on MS-Windows 
> when isatty returns nonzero?

No, the SET_BINARY macro does TRT here.

> Emacs already uses binary-io, by the way; if this were a problem I
> expect we'd already have run into it.

We don't currently use binary-io on Windows.

> >>     /* 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.
> 
> If setting _fmode affects all I/O, why does the WINDOWSNT code bother to 
> call _setmode?

Because setting _fmode only affects the open/fopen calls made _after_
the change.  It cannot affect standard streams that are opened before
'main' runs.

However, I think that we should simply use "rb", "wb", etc. in the
calls to 'fopen', and forget all this _fmode stuff.

(Btw, are there still standard C libraries we care about that don't
support "rb" and "wb"?  If not, we could use these on all platforms,
without any #ifdefs.)

> Conversely, why does make-docfile.c need to set _fmode, 
> if the intent is "Don't put CRs in the DOC file"; shouldn't it suffice 
> to call _setmode on stdout?

The comment is inaccurate: it actually doesn't want to put CRs in any
output files, not just DOC.

> > This logic is flawed: if emacs_get_tty failed, then emacs_set_tty
> > should not be called as well.
> 
> I was just copying the existing logic.  But it's easy to fix while we're 
> at it; revised patch attached.
> 
> > This will not work with Windows isatty, because it doesn't return 1
> > when fd is connected to a console.
> 
> Thanks, I also tried to address that in the attached patch.

Thanks.  I have one comment:

> +int
>  emacs_get_tty (int fd, struct emacs_tty *settings)
>  {
>    /* Retrieve the primary parameters - baud rate, character size, etcetera.  
> */
> @@ -786,15 +787,16 @@
>    HANDLE h = (HANDLE)_get_osfhandle (fd);
>    DWORD console_mode;
>  
> -  if (h && h != INVALID_HANDLE_VALUE)
> +  if (h && h != INVALID_HANDLE_VALUE && GetConsoleMode (h, &console_mode))
>      {
> -      if (GetConsoleMode (h, &console_mode))
> -     settings->main = console_mode;
> +      settings->main = console_mode;
> +      return 0;
>      }
>  #endif       /* WINDOWSNT */
> +  return isatty (fd) - 1;

Do we need this "-1" part now?  It could misfire if isatty does
return 1 some day.