bug#18006: Simplify via set_binary_mode

[Paul Eggert <eggert@cs.ucla.edu>]

[-- Attachment #1: Type: text/plain, Size: 435 bytes --]

Eli Zaretskii wrote:
>> +  return isatty (fd) - 1;
> Do we need this "-1" part now?  It could misfire if isatty does
> return 1 some day.

Sorry, I don't so how it can misfire.  isatty returns 1 on success, 0 on 
failure; whereas the caller returns 0 on success, -1 on failure.  So 
subtracting 1 is the right thing to do, no?

A revised patch, taking your other comments into account, is attached. 
It's relative to trunk bzr 117527.

[-- Attachment #2: binary-io.patch --]
[-- Type: text/plain, Size: 16497 bytes --]

=== modified file 'admin/ChangeLog'
--- admin/ChangeLog	2014-06-30 00:01:51 +0000
+++ admin/ChangeLog	2014-07-14 02:02:15 +0000
@@ -1,3 +1,9 @@
+2014-07-14  Paul Eggert  <eggert@cs.ucla.edu>
+
+	Use binary-io module, O_BINARY, and "b" flag (Bug#18006).
+	* merge-gnulib (GNULIB_MODULES): Add binary-io.  It was already
+	present implicitly; this just makes the dependence explicit.
+
 2014-06-30  Glenn Morris  <rgm@gnu.org>
 
 	* update_autogen: Find loaddefs targets rather than

=== modified file 'admin/merge-gnulib'
--- admin/merge-gnulib	2014-05-17 08:11:31 +0000
+++ admin/merge-gnulib	2014-07-12 20:47:50 +0000
@@ -26,7 +26,7 @@
 GNULIB_URL=git://git.savannah.gnu.org/gnulib.git
 
 GNULIB_MODULES='
-  alloca-opt byteswap c-ctype c-strcase
+  alloca-opt binary-io byteswap c-ctype c-strcase
   careadlinkat close-stream count-one-bits count-trailing-zeros
   crypto/md5 crypto/sha1 crypto/sha256 crypto/sha512
   dtoastr dtotimespec dup2 environ execinfo faccessat

=== modified file 'lib-src/ChangeLog'
--- lib-src/ChangeLog	2014-07-14 00:45:19 +0000
+++ lib-src/ChangeLog	2014-07-14 02:02:15 +0000
@@ -1,3 +1,18 @@
+2014-07-14  Paul Eggert  <eggert@cs.ucla.edu>
+
+	Use binary-io module, O_BINARY, and "b" flag (Bug#18006).
+	* etags.c, hexl.c, make-docfile.c:
+	Include binary-io.h instead of fcntl.h and/or io.h.
+	(main): Use set_binary_mode or SET_BINARY
+	in place of handcrafted code.
+	* etags.c (main) [DOS_NT]:
+	* movemail.c (main) [WINDOWSNT]:
+	Don't mess with _fmode.
+	* etags.c (main, process_file_name, analyse_regex):
+	Use fopen/popen's "b" flag instead.
+	* movemail.c (main, popmail): Use open/lk_open/mkostemp's O_BINARY
+	instead.
+
 2014-07-13  Paul Eggert  <eggert@cs.ucla.edu>
 
 	* make-docfile.c: Simplify a bit, to simplify further refactoring.

=== modified file 'lib-src/etags.c'
--- lib-src/etags.c	2014-07-12 16:26:54 +0000
+++ lib-src/etags.c	2014-07-14 02:02:15 +0000
@@ -105,17 +105,13 @@
 #ifdef MSDOS
 # undef MSDOS
 # define MSDOS true
-# include <fcntl.h>
 # include <sys/param.h>
-# include <io.h>
 #else
 # define MSDOS false
 #endif /* MSDOS */
 
 #ifdef WINDOWSNT
-# include <fcntl.h>
 # include <direct.h>
-# include <io.h>
 # define MAXPATHLEN _MAX_PATH
 # undef HAVE_NTGUI
 # undef  DOS_NT
@@ -131,6 +127,7 @@
 #include <errno.h>
 #include <sys/types.h>
 #include <sys/stat.h>
+#include <binary-io.h>
 #include <c-strcase.h>
 
 #include <assert.h>
@@ -1002,13 +999,8 @@
   linebuffer filename_lb;
   bool help_asked = false;
   ptrdiff_t len;
- char *optstring;
- int opt;
-
-
-#ifdef DOS_NT
-  _fmode = O_BINARY;   /* all of files are treated as binary files */
-#endif /* DOS_NT */
+  char *optstring;
+  int opt;
 
   progname = argv[0];
   nincluded_files = 0;
@@ -1195,15 +1187,10 @@
       if (streq (tagfile, "-"))
 	{
 	  tagf = stdout;
-#ifdef DOS_NT
-	  /* Switch redirected `stdout' to binary mode (setting `_fmode'
-	     doesn't take effect until after `stdout' is already open). */
-	  if (!isatty (fileno (stdout)))
-	    setmode (fileno (stdout), O_BINARY);
-#endif /* DOS_NT */
+	  SET_BINARY (fileno (stdout));
 	}
       else
-	tagf = fopen (tagfile, append_to_tagfile ? "a" : "w");
+	tagf = fopen (tagfile, append_to_tagfile ? "ab" : "wb");
       if (tagf == NULL)
 	pfatal (tagfile);
     }
@@ -1306,7 +1293,7 @@
       append_to_tagfile = true;
     }
 
-  tagf = fopen (tagfile, append_to_tagfile ? "a" : "w");
+  tagf = fopen (tagfile, append_to_tagfile ? "ab" : "wb");
   if (tagf == NULL)
     pfatal (tagfile);
   put_entries (nodehead);	/* write all the tags (CTAGS) */
@@ -1547,11 +1534,11 @@
   if (real_name == compressed_name)
     {
       char *cmd = concat (compr->command, " ", real_name);
-      inf = (FILE *) popen (cmd, "r");
+      inf = popen (cmd, "rb");
       free (cmd);
     }
   else
-    inf = fopen (real_name, "r");
+    inf = fopen (real_name, "rb");
   if (inf == NULL)
     {
       perror (real_name);
@@ -5614,7 +5601,7 @@
 	char *regexfile = regex_arg + 1;
 
 	/* regexfile is a file containing regexps, one per line. */
-	regexfp = fopen (regexfile, "r");
+	regexfp = fopen (regexfile, "rb");
 	if (regexfp == NULL)
 	  pfatal (regexfile);
 	linebuffer_init (&regexbuf);

=== modified file 'lib-src/hexl.c'
--- lib-src/hexl.c	2014-01-01 07:43:34 +0000
+++ lib-src/hexl.c	2014-07-13 05:24:43 +0000
@@ -24,15 +24,8 @@
 
 #include <stdio.h>
 #include <ctype.h>
-#ifdef DOS_NT
-#include <fcntl.h>
-#if __DJGPP__ >= 2
-#include <io.h>
-#endif
-#endif
-#ifdef WINDOWSNT
-#include <io.h>
-#endif
+
+#include <binary-io.h>
 
 #define DEFAULT_GROUPING	0x01
 #define DEFAULT_BASE		16
@@ -155,20 +148,12 @@
 
       if (un_flag)
 	{
-	  char buf[18];
+	  SET_BINARY (fileno (stdout));
 
-#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
 	  for (;;)
 	    {
-	      register int i, c = 0, d;
+	      int i, c = 0, d;
+	      char buf[18];
 
 #define hexchar(x) (isdigit (x) ? x - '0' : x - 'a' + 10)
 
@@ -210,15 +195,7 @@
 	}
       else
 	{
-#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
+	  SET_BINARY (fileno (fp));
 	  address = 0;
 	  string[0] = ' ';
 	  string[17] = '\0';

=== modified file 'lib-src/make-docfile.c'
--- lib-src/make-docfile.c	2014-07-14 00:45:19 +0000
+++ lib-src/make-docfile.c	2014-07-14 00:51:19 +0000
@@ -38,17 +38,16 @@
 
 #include <stdio.h>
 #include <stdlib.h>   /* config.h unconditionally includes this anyway */
-#ifdef MSDOS
-#include <fcntl.h>
-#endif /* MSDOS */
+
 #ifdef WINDOWSNT
 /* Defined to be sys_fopen in ms-w32.h, but only #ifdef emacs, so this
    is really just insurance.  */
 #undef fopen
-#include <fcntl.h>
 #include <direct.h>
 #endif /* WINDOWSNT */
 
+#include <binary-io.h>
+
 #ifdef DOS_NT
 /* Defined to be sys_chdir in ms-w32.h, but only #ifdef emacs, so this
    is really just insurance.
@@ -167,19 +166,7 @@
       ++i;
     }
 
-  /* Don't put CRs in the output 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
-#endif /* MSDOS */
-#ifdef WINDOWSNT
-  _fmode = O_BINARY;
-  _setmode (fileno (stdout), O_BINARY);
-#endif /* WINDOWSNT */
+  set_binary_mode (fileno (stdout), O_BINARY);
 
   if (generate_globals)
     start_globals ();

=== modified file 'lib-src/movemail.c'
--- lib-src/movemail.c	2014-01-01 07:43:34 +0000
+++ lib-src/movemail.c	2014-07-14 02:02:15 +0000
@@ -191,11 +191,6 @@
   uid_t real_gid = getgid ();
   uid_t priv_gid = getegid ();
 
-#ifdef WINDOWSNT
-  /* Ensure all file i/o is in binary mode. */
-  _fmode = _O_BINARY;
-#endif
-
   delete_lockname = 0;
 
   while ((c = getopt (argc, argv, ARGSTR)) != EOF)
@@ -304,7 +299,7 @@
 
 	  memcpy (tempname, inname, inname_dirlen);
 	  strcpy (tempname + inname_dirlen, "EXXXXXX");
-	  desc = mkostemp (tempname, 0);
+	  desc = mkostemp (tempname, O_BINARY);
 	  if (desc < 0)
 	    {
 	      int mkostemp_errno = errno;
@@ -358,12 +353,12 @@
 
 #ifndef MAIL_USE_MMDF
 #ifdef MAIL_USE_SYSTEM_LOCK
-      indesc = open (inname, O_RDWR);
+      indesc = open (inname, O_RDWR | O_BINARY);
 #else  /* if not MAIL_USE_SYSTEM_LOCK */
-      indesc = open (inname, O_RDONLY);
+      indesc = open (inname, O_RDONLY | O_BINARY);
 #endif /* not MAIL_USE_SYSTEM_LOCK */
 #else  /* MAIL_USE_MMDF */
-      indesc = lk_open (inname, O_RDONLY, 0, 0, 10);
+      indesc = lk_open (inname, O_RDONLY | O_BINARY, 0, 0, 10);
 #endif /* MAIL_USE_MMDF */
 
       if (indesc < 0)
@@ -372,7 +367,7 @@
       /* Make sure the user can read the output file.  */
       umask (umask (0) & 0377);
 
-      outdesc = open (outname, O_WRONLY | O_CREAT | O_EXCL, 0666);
+      outdesc = open (outname, O_WRONLY | O_BINARY | O_CREAT | O_EXCL, 0666);
       if (outdesc < 0)
 	pfatal_with_name (outname);
 
@@ -675,7 +670,7 @@
       return EXIT_SUCCESS;
     }
 
-  mbfi = open (outfile, O_WRONLY | O_CREAT | O_EXCL, 0666);
+  mbfi = open (outfile, O_WRONLY | O_BINARY | O_CREAT | O_EXCL, 0666);
   if (mbfi < 0)
     {
       pop_close (server);

=== modified file 'src/ChangeLog'
--- src/ChangeLog	2014-07-13 15:50:35 +0000
+++ src/ChangeLog	2014-07-14 02:02:15 +0000
@@ -1,3 +1,22 @@
+2014-07-14  Paul Eggert  <eggert@cs.ucla.edu>
+
+	Use binary-io module, O_BINARY, and "b" flag (Bug#18006).
+	* callproc.c (create_temp_file): Use mkostemp's O_BINARY flag.
+	* emacs.c [MSDOS]:
+	* emacs.c (main) [DOS_NT]: Don't mess with _fmode.
+	(main) [MSDOS]: Use SET_BINARY instead of setmode.
+	* minibuf.c: Include binary-io.h instead of fcntl.h.
+	(read_minibuf_noninteractive):
+	Use set_binary_mode instead of handcrafted code.
+	Don't call emacs_set_tty if emacs_get_tty failed.
+	* process.c: Include binary-io.h.
+	(Fmake_network_process, network_interface_list)
+	(network_interface_info, server_accept_connection):
+	Use set_binary_mode on newly-created sockets.
+	* sysdep.c, systty.h (emacs_get_tty): Return int, not void.
+	* sysdep.c (emacs_open, emacs_pipe): Use O_BINARY.
+	* w32.c (pipe2): Adjust eassert to include O_BINARY.
+
 2014-07-13  Paul Eggert  <eggert@cs.ucla.edu>
 
 	Improve behavior of 'bzr up; cd src; make -k'.

=== modified file 'src/callproc.c'
--- src/callproc.c	2014-06-08 00:35:27 +0000
+++ src/callproc.c	2014-07-14 02:02:15 +0000
@@ -982,7 +982,7 @@
 
     count = SPECPDL_INDEX ();
     record_unwind_protect_nothing ();
-    fd = mkostemp (tempfile, O_CLOEXEC);
+    fd = mkostemp (tempfile, O_BINARY | O_CLOEXEC);
     if (fd < 0)
       report_file_error ("Failed to open temporary file using pattern",
 			 pattern);

=== modified file 'src/emacs.c'
--- src/emacs.c	2014-06-03 20:08:08 +0000
+++ src/emacs.c	2014-07-14 02:02:15 +0000
@@ -51,6 +51,10 @@
 #include "cygw32.h"
 #endif
 
+#ifdef MSDOS
+#include <binary-io.h>
+#endif
+
 #ifdef HAVE_WINDOW_SYSTEM
 #include TERM_HEADER
 #endif /* HAVE_WINDOW_SYSTEM */
@@ -918,20 +922,10 @@
 
 #endif	/* not SYSTEM_MALLOC */
 
-#if defined (MSDOS) || defined (WINDOWSNT)
-  /* We do all file input/output as binary files.  When we need to translate
-     newlines, we do that manually.  */
-  _fmode = O_BINARY;
-#endif /* MSDOS || WINDOWSNT */
-
 #ifdef MSDOS
-  if (!isatty (fileno (stdin)))
-    setmode (fileno (stdin), O_BINARY);
-  if (!isatty (fileno (stdout)))
-    {
-      fflush (stdout);
-      setmode (fileno (stdout), O_BINARY);
-    }
+  SET_BINARY (fileno (stdin));
+  fflush (stdout);
+  SET_BINARY (fileno (stdout));
 #endif /* MSDOS */
 
   /* Skip initial setlocale if LC_ALL is "C", as it's not needed in that case.

=== modified file 'src/minibuf.c'
--- src/minibuf.c	2014-07-12 07:47:40 +0000
+++ src/minibuf.c	2014-07-13 05:24:43 +0000
@@ -22,9 +22,7 @@
 #include <errno.h>
 #include <stdio.h>
 
-#ifdef WINDOWSNT
-#include <fcntl.h>	/* For O_BINARY, O_TEXT. */
-#endif
+#include <binary-io.h>
 
 #include "lisp.h"
 #include "commands.h"
@@ -231,6 +229,7 @@
   int c;
   unsigned char hide_char = 0;
   struct emacs_tty etty;
+  bool etty_valid;
 
   /* Check, whether we need to suppress echoing.  */
   if (CHARACTERP (Vread_hide_char))
@@ -239,11 +238,9 @@
   /* Manipulate tty.  */
   if (hide_char)
     {
-      emacs_get_tty (fileno (stdin), &etty);
-#ifdef WINDOWSNT
-      if (isatty (fileno (stdin)))
-	_setmode (fileno (stdin), O_BINARY);
-#endif
+      etty_valid = emacs_get_tty (fileno (stdin), &etty) == 0;
+      if (etty_valid)
+	set_binary_mode (fileno (stdin), O_BINARY);
       suppress_echo_on_tty (fileno (stdin));
     }
 
@@ -281,11 +278,11 @@
   if (hide_char)
     {
       fprintf (stdout, "\n");
-      emacs_set_tty (fileno (stdin), &etty, 0);
-#ifdef WINDOWSNT
-      if (isatty (fileno (stdin)))
-	_setmode (fileno (stdin), O_TEXT);
-#endif
+      if (etty_valid)
+	{
+	  emacs_set_tty (fileno (stdin), &etty, 0);
+	  set_binary_mode (fileno (stdin), O_TEXT);
+	}
     }
 
   if (len || c == '\n' || c == '\r')

=== modified file 'src/process.c'
--- src/process.c	2014-07-08 17:13:32 +0000
+++ src/process.c	2014-07-14 02:02:15 +0000
@@ -88,6 +88,7 @@
 #include <pty.h>
 #endif
 
+#include <binary-io.h>
 #include <c-ctype.h>
 #include <sig2str.h>
 #include <verify.h>
@@ -3142,6 +3143,8 @@
 	  continue;
 	}
 
+      set_binary_mode (s, O_BINARY);
+
 #ifdef DATAGRAM_SOCKETS
       if (!is_server && socktype == SOCK_DGRAM)
 	break;
@@ -3535,6 +3538,7 @@
     return Qnil;
   count = SPECPDL_INDEX ();
   record_unwind_protect_int (close_file_unwind, s);
+  set_binary_mode (s, O_BINARY);
 
   do
     {
@@ -3687,6 +3691,7 @@
     return Qnil;
   count = SPECPDL_INDEX ();
   record_unwind_protect_int (close_file_unwind, s);
+  set_binary_mode (s, O_BINARY);
 
   elt = Qnil;
 #if defined (SIOCGIFFLAGS) && defined (HAVE_STRUCT_IFREQ_IFR_FLAGS)
@@ -4134,6 +4139,7 @@
   chan_process[s] = proc;
 
   fcntl (s, F_SETFL, O_NONBLOCK);
+  set_binary_mode (s, O_BINARY);
 
   p = XPROCESS (proc);
 

=== modified file 'src/sysdep.c'
--- src/sysdep.c	2014-07-11 17:55:24 +0000
+++ src/sysdep.c	2014-07-14 02:02:15 +0000
@@ -775,8 +775,9 @@
 /* Getting and setting emacs_tty structures.  */
 
 /* Set *TC to the parameters associated with the terminal FD,
-   or clear it if the parameters are not available.  */
-void
+   or clear it if the parameters are not available.
+   Return 0 on success, -1 (setting errno) on failure.  */
+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;
 #else	/* !DOS_NT */
   /* We have those nifty POSIX tcmumbleattr functions.  */
-  tcgetattr (fd, &settings->main);
+  return tcgetattr (fd, &settings->main);
 #endif
 }
 
@@ -2198,6 +2200,7 @@
 #endif
 
 /* Open FILE for Emacs use, using open flags OFLAG and mode MODE.
+   Use binary I/O on systems that care about text vs binary I/O.
    Arrange for subprograms to not inherit the file descriptor.
    Prefer a method that is multithread-safe, if available.
    Do not fail merely because the open was interrupted by a signal.
@@ -2207,7 +2210,7 @@
 emacs_open (const char *file, int oflags, int mode)
 {
   int fd;
-  oflags |= O_CLOEXEC;
+  oflags |= O_BINARY | O_CLOEXEC;
   while ((fd = open (file, oflags, mode)) < 0 && errno == EINTR)
     QUIT;
   if (! O_CLOEXEC && 0 <= fd)
@@ -2254,7 +2257,7 @@
 #ifdef MSDOS
   return pipe (fd);
 #else  /* !MSDOS */
-  int result = pipe2 (fd, O_CLOEXEC);
+  int result = pipe2 (fd, O_BINARY | O_CLOEXEC);
   if (! O_CLOEXEC && result == 0)
     {
       fcntl (fd[0], F_SETFD, FD_CLOEXEC);

=== modified file 'src/systty.h'
--- src/systty.h	2014-07-11 17:55:24 +0000
+++ src/systty.h	2014-07-12 20:47:50 +0000
@@ -80,7 +80,7 @@
 };
 \f
 /* From sysdep.c or w32.c  */
-extern void emacs_get_tty (int, struct emacs_tty *) EXTERNALLY_VISIBLE;
+extern int emacs_get_tty (int, struct emacs_tty *) EXTERNALLY_VISIBLE;
 extern int emacs_set_tty (int, struct emacs_tty *, bool) EXTERNALLY_VISIBLE;
 extern void suppress_echo_on_tty (int);
 extern int serial_open (Lisp_Object);

=== modified file 'src/w32.c'
--- src/w32.c	2014-07-10 19:09:26 +0000
+++ src/w32.c	2014-07-14 02:02:15 +0000
@@ -882,7 +882,7 @@
 	  g_b_init_set_named_security_info_a = 1;
 	  hm_advapi32 = LoadLibrary ("Advapi32.dll");
 	  s_pfn_Set_Named_Security_InfoA =
-	    (SetNamedSecurityInfoA_Proc) GetProcAddress (hm_advapi32,
+	    (SetNamedSecurityInfoA_Proc) GetProcAddress (hm_advapi32,
 							 "SetNamedSecurityInfoA");
 	}
       if (s_pfn_Set_Named_Security_InfoA == NULL)
@@ -7865,7 +7865,7 @@
   int rc;
   unsigned flags;
 
-  eassert (pipe2_flags == O_CLOEXEC);
+  eassert (pipe2_flags == (O_BINARY | O_CLOEXEC));
 
   /* make pipe handles non-inheritable; when we spawn a child, we
      replace the relevant handle with an inheritable one.  Also put