unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#12881: Assume at least POSIX.1-1988 for fcntl.h
@ 2012-11-14  7:37 Paul Eggert
  2012-11-14 17:33 ` Eli Zaretskii
  2012-11-16  4:38 ` Paul Eggert
  0 siblings, 2 replies; 22+ messages in thread
From: Paul Eggert @ 2012-11-14  7:37 UTC (permalink / raw)
  To: 12881

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

Tags: patch

On POSIXish hosts it's long been safe to assume at least
POSIX.1-1988, as the pre-POSIX platforms died out long ago.
Attached is a patch to simplify Emacs to assume this
for <fcntl.h>.  I haven't tested this on Windows but have
tried to make the Windows port work by renaming its O_NDELAY
flag to O_NONBLOCK, as POSIX standardized the spelling of this
flag to be O_NONBLOCK.  I'll CC: this patch to Eli to give
him a heads-up.  This patch is relative to trunk bzr 110892.

[-- Attachment #2: fcntl.txt --]
[-- Type: text/plain, Size: 17926 bytes --]

=== modified file 'ChangeLog'
--- ChangeLog	2012-11-14 04:55:41 +0000
+++ ChangeLog	2012-11-14 07:26:25 +0000
@@ -1,5 +1,8 @@
 2012-11-14  Paul Eggert  <eggert@cs.ucla.edu>
 
+	Assume POSIX 1003.1-1988 or later for fcntl.h.
+	* lib/gnulib.mk: Regenerate.
+
 	Use faccessat, not access, when checking file permissions (Bug#12632).
 	* .bzrignore: Add lib/fcntl.h.
 	* configure.ac (euidaccess): Remove check; gnulib does this for us now.

=== modified file 'admin/CPP-DEFINES'
--- admin/CPP-DEFINES	2012-11-05 03:18:32 +0000
+++ admin/CPP-DEFINES	2012-11-14 07:26:25 +0000
@@ -420,8 +420,6 @@
 NSIG_MINIMUM
 NULL_DEVICE
 ORDINARY_LINK
-O_RDONLY
-O_RDWR
 PAGESIZE
 PREFER_VSUSP
 PTY_ITERATION

=== modified file 'admin/ChangeLog'
--- admin/ChangeLog	2012-11-14 04:55:41 +0000
+++ admin/ChangeLog	2012-11-14 07:26:25 +0000
@@ -1,5 +1,9 @@
 2012-11-14  Paul Eggert  <eggert@cs.ucla.edu>
 
+	Assume POSIX 1003.1-1988 or later for fcntl.h.
+	* CPP-DEFINES (O_RDONLY, O_RDWR): Remove.
+	* merge-gnulib (GNULIB_MODULES): Add fcntl-h.
+
 	Use faccessat, not access, when checking file permissions (Bug#12632).
 	* merge-gnulib (GNULIB_MODULES): Add faccessat.
 	(GNULIB_TOOL_FLAGS): Avoid at-internal, fchdir, malloc-posix,

=== modified file 'admin/merge-gnulib'
--- admin/merge-gnulib	2012-11-14 04:55:41 +0000
+++ admin/merge-gnulib	2012-11-14 07:26:25 +0000
@@ -29,7 +29,7 @@
   alloca-opt c-ctype c-strcase
   careadlinkat close-stream crypto/md5 crypto/sha1 crypto/sha256 crypto/sha512
   dtoastr dtotimespec dup2 environ execinfo faccessat
-  filemode getloadavg getopt-gnu gettime gettimeofday
+  fcntl-h filemode getloadavg getopt-gnu gettime gettimeofday
   ignore-value intprops largefile lstat
   manywarnings mktime pselect pthread_sigmask readlink
   socklen stat-time stdalign stdarg stdbool stdio

=== modified file 'lib/gnulib.mk'
--- lib/gnulib.mk	2012-11-14 04:55:41 +0000
+++ lib/gnulib.mk	2012-11-14 07:26:25 +0000
@@ -21,7 +21,7 @@
 # the same distribution terms as the rest of that program.
 #
 # Generated by gnulib-tool.
-# Reproduce by: gnulib-tool --import --dir=. --lib=libgnu --source-base=lib --m4-base=m4 --doc-base=doc --tests-base=tests --aux-dir=build-aux --avoid=at-internal --avoid=errno --avoid=fchdir --avoid=fcntl --avoid=fstat --avoid=malloc-posix --avoid=msvc-inval --avoid=msvc-nothrow --avoid=openat-die --avoid=openat-h --avoid=raise --avoid=save-cwd --avoid=select --avoid=sigprocmask --avoid=sys_types --avoid=threadlib --makefile-name=gnulib.mk --conditional-dependencies --no-libtool --macro-prefix=gl --no-vc-files alloca-opt c-ctype c-strcase careadlinkat close-stream crypto/md5 crypto/sha1 crypto/sha256 crypto/sha512 dtoastr dtotimespec dup2 environ execinfo faccessat filemode getloadavg getopt-gnu gettime gettimeofday ignore-value intprops largefile lstat manywarnings mktime pselect pthread_sigmask readlink socklen stat-time stdalign stdarg stdbool stdio strftime strtoimax strtoumax symlink sys_stat sys_time time timer-time timespec-add timespec-sub utimens warnings
+# Reproduce by: gnulib-tool --import --dir=. --lib=libgnu --source-base=lib --m4-base=m4 --doc-base=doc --tests-base=tests --aux-dir=build-aux --avoid=at-internal --avoid=errno --avoid=fchdir --avoid=fcntl --avoid=fstat --avoid=malloc-posix --avoid=msvc-inval --avoid=msvc-nothrow --avoid=openat-die --avoid=openat-h --avoid=raise --avoid=save-cwd --avoid=select --avoid=sigprocmask --avoid=sys_types --avoid=threadlib --makefile-name=gnulib.mk --conditional-dependencies --no-libtool --macro-prefix=gl --no-vc-files alloca-opt c-ctype c-strcase careadlinkat close-stream crypto/md5 crypto/sha1 crypto/sha256 crypto/sha512 dtoastr dtotimespec dup2 environ execinfo faccessat fcntl-h filemode getloadavg getopt-gnu gettime gettimeofday ignore-value intprops largefile lstat manywarnings mktime pselect pthread_sigmask readlink socklen stat-time stdalign stdarg stdbool stdio strftime strtoimax strtoumax symlink sys_stat sys_time time timer-time timespec-add timespec-sub utimens warnings
 
 
 MOSTLYCLEANFILES += core *.stackdump

=== modified file 'nt/ChangeLog'
--- nt/ChangeLog	2012-11-14 04:55:41 +0000
+++ nt/ChangeLog	2012-11-14 07:26:25 +0000
@@ -1,5 +1,9 @@
 2012-11-14  Paul Eggert  <eggert@cs.ucla.edu>
 
+	Assume POSIX 1003.1-1988 or later for fcntl.h.
+	* inc/sys/socket.h (O_NONBLOCK): Rename from O_NDELAY, since the
+	POSIX name for this flag is O_NONBLOCK.  All uses changed.
+
 	Use faccessat, not access, when checking file permissions (Bug#12632).
 	* inc/ms-w32.h (AT_FDCWD, AT_EACCESS): New symbols.
 	(access): Remove.

=== modified file 'nt/inc/sys/socket.h'
--- nt/inc/sys/socket.h	2012-09-30 21:36:42 +0000
+++ nt/inc/sys/socket.h	2012-11-14 07:26:25 +0000
@@ -119,7 +119,7 @@
    an fcntl function, for setting sockets to non-blocking mode.  */
 int fcntl (int s, int cmd, int options);
 #define F_SETFL   4
-#define O_NDELAY  04000
+#define O_NONBLOCK  04000
 
 /* we are providing a real h_errno variable */
 #undef h_errno

=== modified file 'src/ChangeLog'
--- src/ChangeLog	2012-11-14 04:55:41 +0000
+++ src/ChangeLog	2012-11-14 07:26:25 +0000
@@ -1,5 +1,24 @@
 2012-11-14  Paul Eggert  <eggert@cs.ucla.edu>
 
+	Assume POSIX 1003.1-1988 or later for fcntl.h.
+	* callproc.c (relocate_fd): Assume F_DUPFD.
+	* emacs.c, term.c (O_RDWR): Remove.
+	* keyboard.c (tty_read_avail_input): Use O_NONBLOCK rather than
+	O_NDELAY, since O_NONBLOCK is the standard name for this flag.
+	* process.c (NON_BLOCKING_CONNECT, allocate_pty, create_process)
+	(create_pty, Fmake_network_process, server_accept_connection)
+	(wait_reading_process_output, init_process_emacs):
+	Assume O_NONBLOCK.
+	(create_process): Assume FD_CLOEXEC.
+	(create_process, create_pty): Assume O_NOCTTY.
+	* sysdep.c (init_sys_modes, reset_sys_modes): Assume F_SETFL.
+	(reset_sys_modes): Use O_NONBLOCK rather than O_NDELAY.
+	(serial_open): Assume O_NONBLOCK and O_NOCTTY.
+	* term.c (O_NOCTTY): Remove.
+	(init_tty): Assume O_IGNORE_CTTY is defined to 0 on platforms that
+	lack it, since gnulib guarantees this.
+	* w32.c (fcntl): Test for O_NONBLOCK rather than O_NDELAY.
+
 	Use faccessat, not access, when checking file permissions (Bug#12632).
 	This fixes a bug that has been present in Emacs since its creation.
 	It was reported by Chris Torek in 1983 even before GNU Emacs existed,

=== modified file 'src/callproc.c'
--- src/callproc.c	2012-11-14 04:55:41 +0000
+++ src/callproc.c	2012-11-14 07:26:25 +0000
@@ -1317,16 +1317,7 @@
     return fd;
   else
     {
-      int new;
-#ifdef F_DUPFD
-      new = fcntl (fd, F_DUPFD, minfd);
-#else
-      new = dup (fd);
-      if (new != -1)
-	/* Note that we hold the original FD open while we recurse,
-	   to guarantee we'll get a new FD if we need it.  */
-	new = relocate_fd (new, minfd);
-#endif
+      int new = fcntl (fd, F_DUPFD, minfd);
       if (new == -1)
 	{
 	  const char *message_1 = "Error while setting up child: ";

=== modified file 'src/emacs.c'
--- src/emacs.c	2012-11-08 19:12:23 +0000
+++ src/emacs.c	2012-11-14 07:26:25 +0000
@@ -95,10 +95,6 @@
 #include <sys/personality.h>
 #endif
 
-#ifndef O_RDWR
-#define O_RDWR 2
-#endif
-
 static const char emacs_version[] = VERSION;
 static const char emacs_copyright[] = COPYRIGHT;
 

=== modified file 'src/keyboard.c'
--- src/keyboard.c	2012-11-08 09:26:40 +0000
+++ src/keyboard.c	2012-11-14 07:26:25 +0000
@@ -6948,7 +6948,7 @@
 #elif defined USG || defined CYGWIN
   /* Read some input if available, but don't wait.  */
   n_to_read = sizeof cbuf;
-  fcntl (fileno (tty->input), F_SETFL, O_NDELAY);
+  fcntl (fileno (tty->input), F_SETFL, O_NONBLOCK);
 #else
 # error "Cannot read without possibly delaying"
 #endif
@@ -6982,7 +6982,7 @@
     }
   while (
          /* We used to retry the read if it was interrupted.
-            But this does the wrong thing when O_NDELAY causes
+            But this does the wrong thing when O_NONBLOCK causes
             an EAGAIN error.  Does anybody know of a situation
             where a retry is actually needed?  */
 #if 0

=== modified file 'src/process.c'
--- src/process.c	2012-11-14 04:55:41 +0000
+++ src/process.c	2012-11-14 07:26:25 +0000
@@ -208,11 +208,9 @@
 #ifndef NON_BLOCKING_CONNECT
 #ifdef HAVE_SELECT
 #if defined (HAVE_GETPEERNAME) || defined (GNU_LINUX)
-#if O_NONBLOCK || O_NDELAY
 #if defined (EWOULDBLOCK) || defined (EINPROGRESS)
 #define NON_BLOCKING_CONNECT
 #endif /* EWOULDBLOCK || EINPROGRESS */
-#endif /* O_NONBLOCK || O_NDELAY */
 #endif /* HAVE_GETPEERNAME || GNU_LINUX */
 #endif /* HAVE_SELECT */
 #endif /* NON_BLOCKING_CONNECT */
@@ -654,13 +652,7 @@
 #ifdef PTY_OPEN
 	PTY_OPEN;
 #else /* no PTY_OPEN */
-	{
-#  if O_NONBLOCK
-	  fd = emacs_open (pty_name, O_RDWR | O_NONBLOCK, 0);
-#  else
-	  fd = emacs_open (pty_name, O_RDWR | O_NDELAY, 0);
-#  endif
-	}
+	fd = emacs_open (pty_name, O_RDWR | O_NONBLOCK, 0);
 #endif /* no PTY_OPEN */
 
 	if (fd >= 0)
@@ -1598,7 +1590,7 @@
   int inchannel, outchannel;
   pid_t pid;
   int sv[2];
-#if !defined (WINDOWSNT) && defined (FD_CLOEXEC)
+#ifndef WINDOWSNT
   int wait_child_setup[2];
 #endif
 #ifdef SIGCHLD
@@ -1624,13 +1616,9 @@
 #if ! defined (USG) || defined (USG_SUBTTY_WORKS)
       /* On most USG systems it does not work to open the pty's tty here,
 	 then close it and reopen it in the child.  */
-#if O_NOCTTY
       /* Don't let this terminal become our controlling terminal
 	 (in case we don't have one).  */
       forkout = forkin = emacs_open (pty_name, O_RDWR | O_NOCTTY, 0);
-#else
-      forkout = forkin = emacs_open (pty_name, O_RDWR, 0);
-#endif
       if (forkin < 0)
 	report_file_error ("Opening pty", Qnil);
 #else
@@ -1659,7 +1647,7 @@
       forkin = sv[0];
     }
 
-#if !defined (WINDOWSNT) && defined (FD_CLOEXEC)
+#ifndef WINDOWSNT
     {
       int tem;
 
@@ -1678,15 +1666,8 @@
     }
 #endif
 
-#if O_NONBLOCK
   fcntl (inchannel, F_SETFL, O_NONBLOCK);
   fcntl (outchannel, F_SETFL, O_NONBLOCK);
-#else
-#if O_NDELAY
-  fcntl (inchannel, F_SETFL, O_NDELAY);
-  fcntl (outchannel, F_SETFL, O_NDELAY);
-#endif
-#endif
 
   /* Record this as an active process, with its channels.
      As a result, child_setup will close Emacs's side of the pipes.  */
@@ -1845,9 +1826,7 @@
       pid = child_setup (xforkin, xforkout, xforkout,
 			 new_argv, 1, encoded_current_dir);
 #else  /* not WINDOWSNT */
-#ifdef FD_CLOEXEC
       emacs_close (wait_child_setup[0]);
-#endif
       child_setup (xforkin, xforkout, xforkout,
 		   new_argv, 1, encoded_current_dir);
 #endif /* not WINDOWSNT */
@@ -1906,7 +1885,7 @@
 
       pset_tty_name (XPROCESS (process), lisp_pty_name);
 
-#if !defined (WINDOWSNT) && defined (FD_CLOEXEC)
+#ifndef WINDOWSNT
       /* Wait for child_setup to complete in case that vfork is
 	 actually defined as fork.  The descriptor wait_child_setup[1]
 	 of a pipe is closed at the child side either by close-on-exec
@@ -1943,13 +1922,9 @@
 #if ! defined (USG) || defined (USG_SUBTTY_WORKS)
       /* On most USG systems it does not work to open the pty's tty here,
 	 then close it and reopen it in the child.  */
-#if O_NOCTTY
       /* Don't let this terminal become our controlling terminal
 	 (in case we don't have one).  */
       int forkout = emacs_open (pty_name, O_RDWR | O_NOCTTY, 0);
-#else
-      int forkout = emacs_open (pty_name, O_RDWR, 0);
-#endif
       if (forkout < 0)
 	report_file_error ("Opening pty", Qnil);
 #if defined (DONT_REOPEN_PTY)
@@ -1963,15 +1938,8 @@
     }
 #endif /* HAVE_PTYS */
 
-#if O_NONBLOCK
   fcntl (inchannel, F_SETFL, O_NONBLOCK);
   fcntl (outchannel, F_SETFL, O_NONBLOCK);
-#else
-#if O_NDELAY
-  fcntl (inchannel, F_SETFL, O_NDELAY);
-  fcntl (outchannel, F_SETFL, O_NDELAY);
-#endif
-#endif
 
   /* Record this as an active process, with its channels.
      As a result, child_setup will close Emacs's side of the pipes.  */
@@ -2927,13 +2895,9 @@
     {
       /* Don't support network sockets when non-blocking mode is
 	 not available, since a blocked Emacs is not useful.  */
-#if !O_NONBLOCK && !O_NDELAY
-      error ("Network servers not supported");
-#else
       is_server = 1;
       if (TYPE_RANGED_INTEGERP (int, tem))
 	backlog = XINT (tem);
-#endif
     }
 
   /* Make QCaddress an alias for :local (server) or :remote (client).  */
@@ -3193,11 +3157,7 @@
 #ifdef NON_BLOCKING_CONNECT
       if (is_non_blocking_client)
 	{
-#if O_NONBLOCK
 	  ret = fcntl (s, F_SETFL, O_NONBLOCK);
-#else
-	  ret = fcntl (s, F_SETFL, O_NDELAY);
-#endif
 	  if (ret < 0)
 	    {
 	      xerrno = errno;
@@ -3410,13 +3370,7 @@
 
   chan_process[inch] = proc;
 
-#if O_NONBLOCK
   fcntl (inch, F_SETFL, O_NONBLOCK);
-#else
-#if O_NDELAY
-  fcntl (inch, F_SETFL, O_NDELAY);
-#endif
-#endif
 
   p = XPROCESS (proc);
 
@@ -4145,13 +4099,7 @@
 
   chan_process[s] = proc;
 
-#if O_NONBLOCK
   fcntl (s, F_SETFL, O_NONBLOCK);
-#else
-#if O_NDELAY
-  fcntl (s, F_SETFL, O_NDELAY);
-#endif
-#endif
 
   p = XPROCESS (proc);
 
@@ -4847,23 +4795,8 @@
 	      else if (nread == -1 && errno == EWOULDBLOCK)
 		;
 #endif
-	      /* ISC 4.1 defines both EWOULDBLOCK and O_NONBLOCK,
-		 and Emacs uses O_NONBLOCK, so what we get is EAGAIN.  */
-#if O_NONBLOCK
-	      else if (nread == -1 && errno == EAGAIN)
-		;
-#else
-#if O_NDELAY
-	      else if (nread == -1 && errno == EAGAIN)
-		;
-	      /* Note that we cannot distinguish between no input
-		 available now and a closed pipe.
-		 With luck, a closed pipe will be accompanied by
-		 subprocess termination and SIGCHLD.  */
-	      else if (nread == 0 && !NETCONN_P (proc) && !SERIALCONN_P (proc))
-		;
-#endif /* O_NDELAY */
-#endif /* O_NONBLOCK */
+	      else if (nread == -1 && errno == EAGAIN)
+		;
 #ifdef HAVE_PTYS
 	      /* On some OSs with ptys, when the process on one end of
 		 a pty exits, the other end gets an error reading with
@@ -7348,9 +7281,7 @@
 #ifdef HAVE_GETSOCKNAME
    ADD_SUBFEATURE (QCservice, Qt);
 #endif
-#if O_NONBLOCK || O_NDELAY
    ADD_SUBFEATURE (QCserver, Qt);
-#endif
 
    for (sopt = socket_options; sopt->name; sopt++)
      subfeatures = pure_cons (intern_c_string (sopt->name), subfeatures);

=== modified file 'src/sysdep.c'
--- src/sysdep.c	2012-11-14 04:55:41 +0000
+++ src/sysdep.c	2012-11-14 07:26:25 +0000
@@ -1039,8 +1039,7 @@
 #endif
 #endif
 
-#ifdef F_SETFL
-#ifdef F_GETOWN		/* F_SETFL does not imply existence of F_GETOWN */
+#ifdef F_GETOWN
   if (interrupt_input)
     {
       old_fcntl_owner[fileno (tty_out->input)] =
@@ -1058,7 +1057,6 @@
 #endif /* HAVE_GPM */
     }
 #endif /* F_GETOWN */
-#endif /* F_SETFL */
 
 #ifdef _IOFBF
   /* This symbol is defined on recent USG systems.
@@ -1278,8 +1276,7 @@
   fsync (fileno (tty_out->output));
 #endif
 
-#ifdef F_SETFL
-#ifdef F_SETOWN		/* F_SETFL does not imply existence of F_SETOWN */
+#ifdef F_SETOWN
   if (interrupt_input)
     {
       reset_sigio (fileno (tty_out->input));
@@ -1287,11 +1284,8 @@
              old_fcntl_owner[fileno (tty_out->input)]);
     }
 #endif /* F_SETOWN */
-#if O_NDELAY
   fcntl (fileno (tty_out->input), F_SETFL,
-         fcntl (fileno (tty_out->input), F_GETFL, 0) & ~O_NDELAY);
-#endif
-#endif /* F_SETFL */
+         fcntl (fileno (tty_out->input), F_GETFL, 0) & ~O_NONBLOCK);
 
   if (tty_out->old_tty)
     while (emacs_set_tty (fileno (tty_out->input),
@@ -2380,19 +2374,7 @@
 int
 serial_open (char *port)
 {
-  int fd = -1;
-
-  fd = emacs_open ((char*) port,
-		   O_RDWR
-#if O_NONBLOCK
-		   | O_NONBLOCK
-#else
-		   | O_NDELAY
-#endif
-#if O_NOCTTY
-		   | O_NOCTTY
-#endif
-		   , 0);
+  int fd = emacs_open (port, O_RDWR | O_NOCTTY | O_NONBLOCK, 0);
   if (fd < 0)
     {
       error ("Could not open %s: %s",

=== modified file 'src/term.c'
--- src/term.c	2012-11-14 04:55:41 +0000
+++ src/term.c	2012-11-14 07:26:25 +0000
@@ -55,14 +55,6 @@
 #include "xterm.h"
 #endif
 
-#ifndef O_RDWR
-#define O_RDWR 2
-#endif
-
-#ifndef O_NOCTTY
-#define O_NOCTTY 0
-#endif
-
 /* The name of the default console device.  */
 #ifdef WINDOWSNT
 #define DEV_TTY  "CONOUT$"
@@ -2989,22 +2981,18 @@
   set_tty_hooks (terminal);
 
   {
-    int fd;
+    /* Open the terminal device.  */
     FILE *file;
 
-#if O_IGNORE_CTTY
-    if (!ctty)
-      /* Open the terminal device.  Don't recognize it as our
-         controlling terminal, and don't make it the controlling tty
-         if we don't have one at the moment.  */
-      fd = emacs_open (name, O_RDWR | O_IGNORE_CTTY | O_NOCTTY, 0);
-    else
-#endif /* O_IGNORE_CTTY */
-      /* Alas, O_IGNORE_CTTY is a GNU extension that seems to be only
-         defined on Hurd.  On other systems, we need to explicitly
-         dissociate ourselves from the controlling tty when we want to
-         open a frame on the same terminal.  */
-      fd = emacs_open (name, O_RDWR | O_NOCTTY, 0);
+    /* If !ctty, don't recognize it as our controlling terminal, and
+       don't make it the controlling tty if we don't have one now.
+
+       Alas, O_IGNORE_CTTY is a GNU extension that seems to be only
+       defined on Hurd.  On other systems, we need to explicitly
+       dissociate ourselves from the controlling tty when we want to
+       open a frame on the same terminal.  */
+    int flags = O_RDWR | O_NOCTTY | (ctty ? 0 : O_IGNORE_CTTY);
+    int fd = emacs_open (name, flags, 0);
 
     tty->name = xstrdup (name);
     terminal->name = xstrdup (name);
@@ -3023,10 +3011,8 @@
                      name);
       }
 
-#if !O_IGNORE_CTTY
-    if (!ctty)
+    if (!O_IGNORE_CTTY && !ctty)
       dissociate_if_controlling_tty (fd);
-#endif
 
     file = fdopen (fd, "w+");
     tty->input = file;

=== modified file 'src/w32.c'
--- src/w32.c	2012-11-14 04:55:41 +0000
+++ src/w32.c	2012-11-14 07:26:25 +0000
@@ -5849,7 +5849,7 @@
   check_errno ();
   if (fd_info[s].flags & FILE_SOCKET)
     {
-      if (cmd == F_SETFL && options == O_NDELAY)
+      if (cmd == F_SETFL && options == O_NONBLOCK)
 	{
 	  unsigned long nblock = 1;
 	  int rc = pfn_ioctlsocket (SOCK_HANDLE (s), FIONBIO, &nblock);


^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#12881: Assume at least POSIX.1-1988 for fcntl.h
  2012-11-14  7:37 bug#12881: Assume at least POSIX.1-1988 for fcntl.h Paul Eggert
@ 2012-11-14 17:33 ` Eli Zaretskii
  2012-11-15  6:32   ` Paul Eggert
  2012-11-16  4:38 ` Paul Eggert
  1 sibling, 1 reply; 22+ messages in thread
From: Eli Zaretskii @ 2012-11-14 17:33 UTC (permalink / raw)
  To: Paul Eggert; +Cc: 12881

> Date: Tue, 13 Nov 2012 23:37:58 -0800
> From: Paul Eggert <eggert@cs.ucla.edu>
> CC: Eli Zaretskii <eliz@gnu.org>
> 
> On POSIXish hosts it's long been safe to assume at least
> POSIX.1-1988, as the pre-POSIX platforms died out long ago.
> Attached is a patch to simplify Emacs to assume this
> for <fcntl.h>.  I haven't tested this on Windows but have
> tried to make the Windows port work by renaming its O_NDELAY
> flag to O_NONBLOCK, as POSIX standardized the spelling of this
> flag to be O_NONBLOCK.  I'll CC: this patch to Eli to give
> him a heads-up.  This patch is relative to trunk bzr 110892.

Renaming O_NDELAY is a no-brainer, but the patch does much more than
just that.  In several places, it actually changes the code involved
in dealing with the related issues.  Here's one example:

> @@ -4847,23 +4795,8 @@
>  	      else if (nread == -1 && errno == EWOULDBLOCK)
>  		;
>  #endif
> -	      /* ISC 4.1 defines both EWOULDBLOCK and O_NONBLOCK,
> -		 and Emacs uses O_NONBLOCK, so what we get is EAGAIN.  */
> -#if O_NONBLOCK
> -	      else if (nread == -1 && errno == EAGAIN)
> -		;
> -#else
> -#if O_NDELAY
> -	      else if (nread == -1 && errno == EAGAIN)
> -		;
> -	      /* Note that we cannot distinguish between no input
> -		 available now and a closed pipe.
> -		 With luck, a closed pipe will be accompanied by
> -		 subprocess termination and SIGCHLD.  */
> -	      else if (nread == 0 && !NETCONN_P (proc) && !SERIALCONN_P (proc))
> -		;
> -#endif /* O_NDELAY */
> -#endif /* O_NONBLOCK */
> +	      else if (nread == -1 && errno == EAGAIN)
> +		;

Here, the code that was left is identical to the one used by platforms
with O_NONBLOCK, but the code used by platforms with O_NDELAY was
different in non-trivial ways.

Another potential problem is that the Windows emulation of fcntl only
works for sockets, whereas O_NONBLOCK is now used in other system
calls, like in emacs_open etc.

This patch also modifies code unrelated to O_NONBLOCK, like this one:

> --- src/callproc.c	2012-11-14 04:55:41 +0000
> +++ src/callproc.c	2012-11-14 07:26:25 +0000
> @@ -1317,16 +1317,7 @@
>      return fd;
>    else
>      {
> -      int new;
> -#ifdef F_DUPFD
> -      new = fcntl (fd, F_DUPFD, minfd);
> -#else
> -      new = dup (fd);
> -      if (new != -1)
> -	/* Note that we hold the original FD open while we recurse,
> -	   to guarantee we'll get a new FD if we need it.  */
> -	new = relocate_fd (new, minfd);
> -#endif
> +      int new = fcntl (fd, F_DUPFD, minfd);
>        if (new == -1)
>  	{
>  	  const char *message_1 = "Error while setting up child: ";

Likewise with O_NOCTTY.

It's possible that you've already analyzed all these places, and they
either aren't getting compiled on Windows or are no-ops.  If so,
please tell the details.  Failing that, Someone(TM) will have to do
the footwork of making sure these changes don't break non-Posix
platforms.





^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#12881: Assume at least POSIX.1-1988 for fcntl.h
  2012-11-14 17:33 ` Eli Zaretskii
@ 2012-11-15  6:32   ` Paul Eggert
  2012-11-15 17:42     ` Eli Zaretskii
  0 siblings, 1 reply; 22+ messages in thread
From: Paul Eggert @ 2012-11-15  6:32 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 12881

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

On 11/14/2012 09:33 AM, Eli Zaretskii wrote:

> In several places, it actually changes the code involved
> in dealing with the related issues.  Here's one example:

As far as I know, that is the only example involving O_NONBLOCK.

> Here, the code that was left is identical to the one used by platforms
> with O_NONBLOCK, but the code used by platforms with O_NDELAY was
> different in non-trivial ways.

Do those differences matter for Microsoft platforms?  If so, it's
simple to keep them just for DOS_NT, with the following further change:

=== modified file 'src/process.c'
--- src/process.c	2012-11-14 07:26:25 +0000
+++ src/process.c	2012-11-15 06:19:46 +0000
@@ -4797,6 +4797,14 @@
 #endif
 	      else if (nread == -1 && errno == EAGAIN)
 		;
+#ifdef DOS_NT
+	      /* Note that we cannot distinguish between no input
+		 available now and a closed pipe.
+		 With luck, a closed pipe will be accompanied by
+		 subprocess termination and SIGCHLD.  */
+	      else if (nread == 0 && !NETCONN_P (proc) && !SERIALCONN_P (proc))
+		;
+#endif
 #ifdef HAVE_PTYS
 	      /* On some OSs with ptys, when the process on one end of
 		 a pty exits, the other end gets an error reading with


> Another potential problem is that the Windows emulation of fcntl only
> works for sockets, whereas O_NONBLOCK is now used in other system
> calls, like in emacs_open etc.

emacs_open is the only other system call where it's used, and there it's
used only in code that is "#ifndef DOS_NT", so it should be OK.

> This patch also modifies code unrelated to O_NONBLOCK, like this one:

That's OK, as this patch is about all symbols defined by fcntl.h
in POSIX.1-1988, not just about O_NONBLOCK.

> Failing that, Someone(TM) will have to do the footwork of making
> sure these changes don't break non-Posix platforms.

The only non-POSIX platform are the Microsoft ones, right?
And if the above analysis is right, we should be OK.

Attached is a revised patch, relative to trunk bzr 110904,
and with the above extra change.


[-- Attachment #2: fcntl.txt --]
[-- Type: text/plain, Size: 18006 bytes --]

=== modified file 'ChangeLog'
--- ChangeLog	2012-11-14 04:55:41 +0000
+++ ChangeLog	2012-11-15 06:03:45 +0000
@@ -1,3 +1,8 @@
+2012-11-15  Paul Eggert  <eggert@cs.ucla.edu>
+
+	Assume POSIX 1003.1-1988 or later for fcntl.h (Bug#12881).
+	* lib/gnulib.mk: Regenerate.
+
 2012-11-14  Paul Eggert  <eggert@cs.ucla.edu>
 
 	Use faccessat, not access, when checking file permissions (Bug#12632).

=== modified file 'admin/CPP-DEFINES'
--- admin/CPP-DEFINES	2012-11-05 03:18:32 +0000
+++ admin/CPP-DEFINES	2012-11-14 07:26:25 +0000
@@ -420,8 +420,6 @@
 NSIG_MINIMUM
 NULL_DEVICE
 ORDINARY_LINK
-O_RDONLY
-O_RDWR
 PAGESIZE
 PREFER_VSUSP
 PTY_ITERATION

=== modified file 'admin/ChangeLog'
--- admin/ChangeLog	2012-11-14 04:55:41 +0000
+++ admin/ChangeLog	2012-11-15 06:03:45 +0000
@@ -1,3 +1,9 @@
+2012-11-15  Paul Eggert  <eggert@cs.ucla.edu>
+
+	Assume POSIX 1003.1-1988 or later for fcntl.h (Bug#12881).
+	* CPP-DEFINES (O_RDONLY, O_RDWR): Remove.
+	* merge-gnulib (GNULIB_MODULES): Add fcntl-h.
+
 2012-11-14  Paul Eggert  <eggert@cs.ucla.edu>
 
 	Use faccessat, not access, when checking file permissions (Bug#12632).

=== modified file 'admin/merge-gnulib'
--- admin/merge-gnulib	2012-11-14 04:55:41 +0000
+++ admin/merge-gnulib	2012-11-14 07:26:25 +0000
@@ -29,7 +29,7 @@
   alloca-opt c-ctype c-strcase
   careadlinkat close-stream crypto/md5 crypto/sha1 crypto/sha256 crypto/sha512
   dtoastr dtotimespec dup2 environ execinfo faccessat
-  filemode getloadavg getopt-gnu gettime gettimeofday
+  fcntl-h filemode getloadavg getopt-gnu gettime gettimeofday
   ignore-value intprops largefile lstat
   manywarnings mktime pselect pthread_sigmask readlink
   socklen stat-time stdalign stdarg stdbool stdio

=== modified file 'lib/gnulib.mk'
--- lib/gnulib.mk	2012-11-14 04:55:41 +0000
+++ lib/gnulib.mk	2012-11-14 07:26:25 +0000
@@ -21,7 +21,7 @@
 # the same distribution terms as the rest of that program.
 #
 # Generated by gnulib-tool.
-# Reproduce by: gnulib-tool --import --dir=. --lib=libgnu --source-base=lib --m4-base=m4 --doc-base=doc --tests-base=tests --aux-dir=build-aux --avoid=at-internal --avoid=errno --avoid=fchdir --avoid=fcntl --avoid=fstat --avoid=malloc-posix --avoid=msvc-inval --avoid=msvc-nothrow --avoid=openat-die --avoid=openat-h --avoid=raise --avoid=save-cwd --avoid=select --avoid=sigprocmask --avoid=sys_types --avoid=threadlib --makefile-name=gnulib.mk --conditional-dependencies --no-libtool --macro-prefix=gl --no-vc-files alloca-opt c-ctype c-strcase careadlinkat close-stream crypto/md5 crypto/sha1 crypto/sha256 crypto/sha512 dtoastr dtotimespec dup2 environ execinfo faccessat filemode getloadavg getopt-gnu gettime gettimeofday ignore-value intprops largefile lstat manywarnings mktime pselect pthread_sigmask readlink socklen stat-time stdalign stdarg stdbool stdio strftime strtoimax strtoumax symlink sys_stat sys_time time timer-time timespec-add timespec-sub utimens warnings
+# Reproduce by: gnulib-tool --import --dir=. --lib=libgnu --source-base=lib --m4-base=m4 --doc-base=doc --tests-base=tests --aux-dir=build-aux --avoid=at-internal --avoid=errno --avoid=fchdir --avoid=fcntl --avoid=fstat --avoid=malloc-posix --avoid=msvc-inval --avoid=msvc-nothrow --avoid=openat-die --avoid=openat-h --avoid=raise --avoid=save-cwd --avoid=select --avoid=sigprocmask --avoid=sys_types --avoid=threadlib --makefile-name=gnulib.mk --conditional-dependencies --no-libtool --macro-prefix=gl --no-vc-files alloca-opt c-ctype c-strcase careadlinkat close-stream crypto/md5 crypto/sha1 crypto/sha256 crypto/sha512 dtoastr dtotimespec dup2 environ execinfo faccessat fcntl-h filemode getloadavg getopt-gnu gettime gettimeofday ignore-value intprops largefile lstat manywarnings mktime pselect pthread_sigmask readlink socklen stat-time stdalign stdarg stdbool stdio strftime strtoimax strtoumax symlink sys_stat sys_time time timer-time timespec-add timespec-sub utimens warnings
 
 
 MOSTLYCLEANFILES += core *.stackdump

=== modified file 'nt/ChangeLog'
--- nt/ChangeLog	2012-11-14 17:22:55 +0000
+++ nt/ChangeLog	2012-11-15 06:03:45 +0000
@@ -1,3 +1,9 @@
+2012-11-15  Paul Eggert  <eggert@cs.ucla.edu>
+
+	Assume POSIX 1003.1-1988 or later for fcntl.h (Bug#12881).
+	* inc/sys/socket.h (O_NONBLOCK): Rename from O_NDELAY, since the
+	POSIX name for this flag is O_NONBLOCK.  All uses changed.
+
 2012-11-14  Eli Zaretskii  <eliz@gnu.org>
 
 	* inc/unistd.h (faccessat): Add prototype.

=== modified file 'nt/inc/sys/socket.h'
--- nt/inc/sys/socket.h	2012-09-30 21:36:42 +0000
+++ nt/inc/sys/socket.h	2012-11-14 07:26:25 +0000
@@ -119,7 +119,7 @@
    an fcntl function, for setting sockets to non-blocking mode.  */
 int fcntl (int s, int cmd, int options);
 #define F_SETFL   4
-#define O_NDELAY  04000
+#define O_NONBLOCK  04000
 
 /* we are providing a real h_errno variable */
 #undef h_errno

=== modified file 'src/ChangeLog'
--- src/ChangeLog	2012-11-15 00:41:32 +0000
+++ src/ChangeLog	2012-11-15 06:19:46 +0000
@@ -1,5 +1,27 @@
 2012-11-15  Paul Eggert  <eggert@cs.ucla.edu>
 
+	Assume POSIX 1003.1-1988 or later for fcntl.h (Bug#12881).
+	* callproc.c (relocate_fd): Assume F_DUPFD.
+	* emacs.c, term.c (O_RDWR): Remove.
+	* keyboard.c (tty_read_avail_input): Use O_NONBLOCK rather than
+	O_NDELAY, since O_NONBLOCK is the standard name for this flag.
+	* process.c (NON_BLOCKING_CONNECT, allocate_pty, create_process)
+	(create_pty, Fmake_network_process, server_accept_connection)
+	(wait_reading_process_output, init_process_emacs):
+	Assume O_NONBLOCK.
+	(wait_reading_process_output): Put in a special case for DOS_NT,
+	to mimick the older behavior where it had O_NDELAY but not O_NONBLOCK.
+	It's not clear this is needed, but it's a more-conservative change.
+	(create_process): Assume FD_CLOEXEC.
+	(create_process, create_pty): Assume O_NOCTTY.
+	* sysdep.c (init_sys_modes, reset_sys_modes): Assume F_SETFL.
+	(reset_sys_modes): Use O_NONBLOCK rather than O_NDELAY.
+	(serial_open): Assume O_NONBLOCK and O_NOCTTY.
+	* term.c (O_NOCTTY): Remove.
+	(init_tty): Assume O_IGNORE_CTTY is defined to 0 on platforms that
+	lack it, since gnulib guarantees this.
+	* w32.c (fcntl): Test for O_NONBLOCK rather than O_NDELAY.
+
 	* eval.c (mark_backtrace) [BYTE_MARK_STACK]: Remove stray '*'.
 	This follows up on the 2012-09-29 patch that removed indirection
 	for the 'function' field.  Reported by Sergey Vinokurov in

=== modified file 'src/callproc.c'
--- src/callproc.c	2012-11-14 04:55:41 +0000
+++ src/callproc.c	2012-11-14 07:26:25 +0000
@@ -1317,16 +1317,7 @@
     return fd;
   else
     {
-      int new;
-#ifdef F_DUPFD
-      new = fcntl (fd, F_DUPFD, minfd);
-#else
-      new = dup (fd);
-      if (new != -1)
-	/* Note that we hold the original FD open while we recurse,
-	   to guarantee we'll get a new FD if we need it.  */
-	new = relocate_fd (new, minfd);
-#endif
+      int new = fcntl (fd, F_DUPFD, minfd);
       if (new == -1)
 	{
 	  const char *message_1 = "Error while setting up child: ";

=== modified file 'src/emacs.c'
--- src/emacs.c	2012-11-08 19:12:23 +0000
+++ src/emacs.c	2012-11-14 07:26:25 +0000
@@ -95,10 +95,6 @@
 #include <sys/personality.h>
 #endif
 
-#ifndef O_RDWR
-#define O_RDWR 2
-#endif
-
 static const char emacs_version[] = VERSION;
 static const char emacs_copyright[] = COPYRIGHT;
 

=== modified file 'src/keyboard.c'
--- src/keyboard.c	2012-11-08 09:26:40 +0000
+++ src/keyboard.c	2012-11-14 07:26:25 +0000
@@ -6948,7 +6948,7 @@
 #elif defined USG || defined CYGWIN
   /* Read some input if available, but don't wait.  */
   n_to_read = sizeof cbuf;
-  fcntl (fileno (tty->input), F_SETFL, O_NDELAY);
+  fcntl (fileno (tty->input), F_SETFL, O_NONBLOCK);
 #else
 # error "Cannot read without possibly delaying"
 #endif
@@ -6982,7 +6982,7 @@
     }
   while (
          /* We used to retry the read if it was interrupted.
-            But this does the wrong thing when O_NDELAY causes
+            But this does the wrong thing when O_NONBLOCK causes
             an EAGAIN error.  Does anybody know of a situation
             where a retry is actually needed?  */
 #if 0

=== modified file 'src/process.c'
--- src/process.c	2012-11-14 04:55:41 +0000
+++ src/process.c	2012-11-15 06:19:46 +0000
@@ -208,11 +208,9 @@
 #ifndef NON_BLOCKING_CONNECT
 #ifdef HAVE_SELECT
 #if defined (HAVE_GETPEERNAME) || defined (GNU_LINUX)
-#if O_NONBLOCK || O_NDELAY
 #if defined (EWOULDBLOCK) || defined (EINPROGRESS)
 #define NON_BLOCKING_CONNECT
 #endif /* EWOULDBLOCK || EINPROGRESS */
-#endif /* O_NONBLOCK || O_NDELAY */
 #endif /* HAVE_GETPEERNAME || GNU_LINUX */
 #endif /* HAVE_SELECT */
 #endif /* NON_BLOCKING_CONNECT */
@@ -654,13 +652,7 @@
 #ifdef PTY_OPEN
 	PTY_OPEN;
 #else /* no PTY_OPEN */
-	{
-#  if O_NONBLOCK
-	  fd = emacs_open (pty_name, O_RDWR | O_NONBLOCK, 0);
-#  else
-	  fd = emacs_open (pty_name, O_RDWR | O_NDELAY, 0);
-#  endif
-	}
+	fd = emacs_open (pty_name, O_RDWR | O_NONBLOCK, 0);
 #endif /* no PTY_OPEN */
 
 	if (fd >= 0)
@@ -1598,7 +1590,7 @@
   int inchannel, outchannel;
   pid_t pid;
   int sv[2];
-#if !defined (WINDOWSNT) && defined (FD_CLOEXEC)
+#ifndef WINDOWSNT
   int wait_child_setup[2];
 #endif
 #ifdef SIGCHLD
@@ -1624,13 +1616,9 @@
 #if ! defined (USG) || defined (USG_SUBTTY_WORKS)
       /* On most USG systems it does not work to open the pty's tty here,
 	 then close it and reopen it in the child.  */
-#if O_NOCTTY
       /* Don't let this terminal become our controlling terminal
 	 (in case we don't have one).  */
       forkout = forkin = emacs_open (pty_name, O_RDWR | O_NOCTTY, 0);
-#else
-      forkout = forkin = emacs_open (pty_name, O_RDWR, 0);
-#endif
       if (forkin < 0)
 	report_file_error ("Opening pty", Qnil);
 #else
@@ -1659,7 +1647,7 @@
       forkin = sv[0];
     }
 
-#if !defined (WINDOWSNT) && defined (FD_CLOEXEC)
+#ifndef WINDOWSNT
     {
       int tem;
 
@@ -1678,15 +1666,8 @@
     }
 #endif
 
-#if O_NONBLOCK
   fcntl (inchannel, F_SETFL, O_NONBLOCK);
   fcntl (outchannel, F_SETFL, O_NONBLOCK);
-#else
-#if O_NDELAY
-  fcntl (inchannel, F_SETFL, O_NDELAY);
-  fcntl (outchannel, F_SETFL, O_NDELAY);
-#endif
-#endif
 
   /* Record this as an active process, with its channels.
      As a result, child_setup will close Emacs's side of the pipes.  */
@@ -1845,9 +1826,7 @@
       pid = child_setup (xforkin, xforkout, xforkout,
 			 new_argv, 1, encoded_current_dir);
 #else  /* not WINDOWSNT */
-#ifdef FD_CLOEXEC
       emacs_close (wait_child_setup[0]);
-#endif
       child_setup (xforkin, xforkout, xforkout,
 		   new_argv, 1, encoded_current_dir);
 #endif /* not WINDOWSNT */
@@ -1906,7 +1885,7 @@
 
       pset_tty_name (XPROCESS (process), lisp_pty_name);
 
-#if !defined (WINDOWSNT) && defined (FD_CLOEXEC)
+#ifndef WINDOWSNT
       /* Wait for child_setup to complete in case that vfork is
 	 actually defined as fork.  The descriptor wait_child_setup[1]
 	 of a pipe is closed at the child side either by close-on-exec
@@ -1943,13 +1922,9 @@
 #if ! defined (USG) || defined (USG_SUBTTY_WORKS)
       /* On most USG systems it does not work to open the pty's tty here,
 	 then close it and reopen it in the child.  */
-#if O_NOCTTY
       /* Don't let this terminal become our controlling terminal
 	 (in case we don't have one).  */
       int forkout = emacs_open (pty_name, O_RDWR | O_NOCTTY, 0);
-#else
-      int forkout = emacs_open (pty_name, O_RDWR, 0);
-#endif
       if (forkout < 0)
 	report_file_error ("Opening pty", Qnil);
 #if defined (DONT_REOPEN_PTY)
@@ -1963,15 +1938,8 @@
     }
 #endif /* HAVE_PTYS */
 
-#if O_NONBLOCK
   fcntl (inchannel, F_SETFL, O_NONBLOCK);
   fcntl (outchannel, F_SETFL, O_NONBLOCK);
-#else
-#if O_NDELAY
-  fcntl (inchannel, F_SETFL, O_NDELAY);
-  fcntl (outchannel, F_SETFL, O_NDELAY);
-#endif
-#endif
 
   /* Record this as an active process, with its channels.
      As a result, child_setup will close Emacs's side of the pipes.  */
@@ -2927,13 +2895,9 @@
     {
       /* Don't support network sockets when non-blocking mode is
 	 not available, since a blocked Emacs is not useful.  */
-#if !O_NONBLOCK && !O_NDELAY
-      error ("Network servers not supported");
-#else
       is_server = 1;
       if (TYPE_RANGED_INTEGERP (int, tem))
 	backlog = XINT (tem);
-#endif
     }
 
   /* Make QCaddress an alias for :local (server) or :remote (client).  */
@@ -3193,11 +3157,7 @@
 #ifdef NON_BLOCKING_CONNECT
       if (is_non_blocking_client)
 	{
-#if O_NONBLOCK
 	  ret = fcntl (s, F_SETFL, O_NONBLOCK);
-#else
-	  ret = fcntl (s, F_SETFL, O_NDELAY);
-#endif
 	  if (ret < 0)
 	    {
 	      xerrno = errno;
@@ -3410,13 +3370,7 @@
 
   chan_process[inch] = proc;
 
-#if O_NONBLOCK
   fcntl (inch, F_SETFL, O_NONBLOCK);
-#else
-#if O_NDELAY
-  fcntl (inch, F_SETFL, O_NDELAY);
-#endif
-#endif
 
   p = XPROCESS (proc);
 
@@ -4145,13 +4099,7 @@
 
   chan_process[s] = proc;
 
-#if O_NONBLOCK
   fcntl (s, F_SETFL, O_NONBLOCK);
-#else
-#if O_NDELAY
-  fcntl (s, F_SETFL, O_NDELAY);
-#endif
-#endif
 
   p = XPROCESS (proc);
 
@@ -4847,23 +4795,16 @@
 	      else if (nread == -1 && errno == EWOULDBLOCK)
 		;
 #endif
-	      /* ISC 4.1 defines both EWOULDBLOCK and O_NONBLOCK,
-		 and Emacs uses O_NONBLOCK, so what we get is EAGAIN.  */
-#if O_NONBLOCK
-	      else if (nread == -1 && errno == EAGAIN)
-		;
-#else
-#if O_NDELAY
-	      else if (nread == -1 && errno == EAGAIN)
-		;
+	      else if (nread == -1 && errno == EAGAIN)
+		;
+#ifdef DOS_NT
 	      /* Note that we cannot distinguish between no input
 		 available now and a closed pipe.
 		 With luck, a closed pipe will be accompanied by
 		 subprocess termination and SIGCHLD.  */
 	      else if (nread == 0 && !NETCONN_P (proc) && !SERIALCONN_P (proc))
 		;
-#endif /* O_NDELAY */
-#endif /* O_NONBLOCK */
+#endif
 #ifdef HAVE_PTYS
 	      /* On some OSs with ptys, when the process on one end of
 		 a pty exits, the other end gets an error reading with
@@ -7348,9 +7289,7 @@
 #ifdef HAVE_GETSOCKNAME
    ADD_SUBFEATURE (QCservice, Qt);
 #endif
-#if O_NONBLOCK || O_NDELAY
    ADD_SUBFEATURE (QCserver, Qt);
-#endif
 
    for (sopt = socket_options; sopt->name; sopt++)
      subfeatures = pure_cons (intern_c_string (sopt->name), subfeatures);

=== modified file 'src/sysdep.c'
--- src/sysdep.c	2012-11-14 04:55:41 +0000
+++ src/sysdep.c	2012-11-14 07:26:25 +0000
@@ -1039,8 +1039,7 @@
 #endif
 #endif
 
-#ifdef F_SETFL
-#ifdef F_GETOWN		/* F_SETFL does not imply existence of F_GETOWN */
+#ifdef F_GETOWN
   if (interrupt_input)
     {
       old_fcntl_owner[fileno (tty_out->input)] =
@@ -1058,7 +1057,6 @@
 #endif /* HAVE_GPM */
     }
 #endif /* F_GETOWN */
-#endif /* F_SETFL */
 
 #ifdef _IOFBF
   /* This symbol is defined on recent USG systems.
@@ -1278,8 +1276,7 @@
   fsync (fileno (tty_out->output));
 #endif
 
-#ifdef F_SETFL
-#ifdef F_SETOWN		/* F_SETFL does not imply existence of F_SETOWN */
+#ifdef F_SETOWN
   if (interrupt_input)
     {
       reset_sigio (fileno (tty_out->input));
@@ -1287,11 +1284,8 @@
              old_fcntl_owner[fileno (tty_out->input)]);
     }
 #endif /* F_SETOWN */
-#if O_NDELAY
   fcntl (fileno (tty_out->input), F_SETFL,
-         fcntl (fileno (tty_out->input), F_GETFL, 0) & ~O_NDELAY);
-#endif
-#endif /* F_SETFL */
+         fcntl (fileno (tty_out->input), F_GETFL, 0) & ~O_NONBLOCK);
 
   if (tty_out->old_tty)
     while (emacs_set_tty (fileno (tty_out->input),
@@ -2380,19 +2374,7 @@
 int
 serial_open (char *port)
 {
-  int fd = -1;
-
-  fd = emacs_open ((char*) port,
-		   O_RDWR
-#if O_NONBLOCK
-		   | O_NONBLOCK
-#else
-		   | O_NDELAY
-#endif
-#if O_NOCTTY
-		   | O_NOCTTY
-#endif
-		   , 0);
+  int fd = emacs_open (port, O_RDWR | O_NOCTTY | O_NONBLOCK, 0);
   if (fd < 0)
     {
       error ("Could not open %s: %s",

=== modified file 'src/term.c'
--- src/term.c	2012-11-14 04:55:41 +0000
+++ src/term.c	2012-11-14 07:26:25 +0000
@@ -55,14 +55,6 @@
 #include "xterm.h"
 #endif
 
-#ifndef O_RDWR
-#define O_RDWR 2
-#endif
-
-#ifndef O_NOCTTY
-#define O_NOCTTY 0
-#endif
-
 /* The name of the default console device.  */
 #ifdef WINDOWSNT
 #define DEV_TTY  "CONOUT$"
@@ -2989,22 +2981,18 @@
   set_tty_hooks (terminal);
 
   {
-    int fd;
+    /* Open the terminal device.  */
     FILE *file;
 
-#if O_IGNORE_CTTY
-    if (!ctty)
-      /* Open the terminal device.  Don't recognize it as our
-         controlling terminal, and don't make it the controlling tty
-         if we don't have one at the moment.  */
-      fd = emacs_open (name, O_RDWR | O_IGNORE_CTTY | O_NOCTTY, 0);
-    else
-#endif /* O_IGNORE_CTTY */
-      /* Alas, O_IGNORE_CTTY is a GNU extension that seems to be only
-         defined on Hurd.  On other systems, we need to explicitly
-         dissociate ourselves from the controlling tty when we want to
-         open a frame on the same terminal.  */
-      fd = emacs_open (name, O_RDWR | O_NOCTTY, 0);
+    /* If !ctty, don't recognize it as our controlling terminal, and
+       don't make it the controlling tty if we don't have one now.
+
+       Alas, O_IGNORE_CTTY is a GNU extension that seems to be only
+       defined on Hurd.  On other systems, we need to explicitly
+       dissociate ourselves from the controlling tty when we want to
+       open a frame on the same terminal.  */
+    int flags = O_RDWR | O_NOCTTY | (ctty ? 0 : O_IGNORE_CTTY);
+    int fd = emacs_open (name, flags, 0);
 
     tty->name = xstrdup (name);
     terminal->name = xstrdup (name);
@@ -3023,10 +3011,8 @@
                      name);
       }
 
-#if !O_IGNORE_CTTY
-    if (!ctty)
+    if (!O_IGNORE_CTTY && !ctty)
       dissociate_if_controlling_tty (fd);
-#endif
 
     file = fdopen (fd, "w+");
     tty->input = file;

=== modified file 'src/w32.c'
--- src/w32.c	2012-11-14 17:22:55 +0000
+++ src/w32.c	2012-11-15 05:41:09 +0000
@@ -5853,7 +5853,7 @@
   check_errno ();
   if (fd_info[s].flags & FILE_SOCKET)
     {
-      if (cmd == F_SETFL && options == O_NDELAY)
+      if (cmd == F_SETFL && options == O_NONBLOCK)
 	{
 	  unsigned long nblock = 1;
 	  int rc = pfn_ioctlsocket (SOCK_HANDLE (s), FIONBIO, &nblock);


^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#12881: Assume at least POSIX.1-1988 for fcntl.h
  2012-11-15  6:32   ` Paul Eggert
@ 2012-11-15 17:42     ` Eli Zaretskii
  2012-11-15 20:07       ` Paul Eggert
  0 siblings, 1 reply; 22+ messages in thread
From: Eli Zaretskii @ 2012-11-15 17:42 UTC (permalink / raw)
  To: Paul Eggert; +Cc: 12881

> Date: Wed, 14 Nov 2012 22:32:07 -0800
> From: Paul Eggert <eggert@cs.ucla.edu>
> CC: 12881@debbugs.gnu.org
> 
> > In several places, it actually changes the code involved
> > in dealing with the related issues.  Here's one example:
> 
> As far as I know, that is the only example involving O_NONBLOCK.
> 
> > Here, the code that was left is identical to the one used by platforms
> > with O_NONBLOCK, but the code used by platforms with O_NDELAY was
> > different in non-trivial ways.
> 
> Do those differences matter for Microsoft platforms?

I don't know, I will have to take a closer look.

> > Another potential problem is that the Windows emulation of fcntl only
> > works for sockets, whereas O_NONBLOCK is now used in other system
> > calls, like in emacs_open etc.
> 
> emacs_open is the only other system call where it's used, and there it's
> used only in code that is "#ifndef DOS_NT", so it should be OK.
> 
> > This patch also modifies code unrelated to O_NONBLOCK, like this one:
> 
> That's OK, as this patch is about all symbols defined by fcntl.h
> in POSIX.1-1988, not just about O_NONBLOCK.

Then perhaps the problem is much smaller than I thought.

> Attached is a revised patch, relative to trunk bzr 110904,
> and with the above extra change.

Thanks, please hold on to it for a few days, until I or someone else
will have time to study it more closely.





^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#12881: Assume at least POSIX.1-1988 for fcntl.h
  2012-11-15 17:42     ` Eli Zaretskii
@ 2012-11-15 20:07       ` Paul Eggert
  2012-11-16  9:49         ` Eli Zaretskii
  0 siblings, 1 reply; 22+ messages in thread
From: Paul Eggert @ 2012-11-15 20:07 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 12881

On 11/15/12 09:42, Eli Zaretskii wrote:
> Thanks, please hold on to it for a few days, until I or someone else
> will have time to study it more closely.

I'll hold onto it for a bit, but there's no need for a close review
process here.  The code works fine on POSIXish hosts and we've done a
quick scan for plausible failures on Windows hosts and have come up
dry.  Any problems that come up on Windows will likely be either
easy-to-fix compilation issues, or further simplifications of the code
that will not fix bugs.





^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#12881: Assume at least POSIX.1-1988 for fcntl.h
  2012-11-14  7:37 bug#12881: Assume at least POSIX.1-1988 for fcntl.h Paul Eggert
  2012-11-14 17:33 ` Eli Zaretskii
@ 2012-11-16  4:38 ` Paul Eggert
  1 sibling, 0 replies; 22+ messages in thread
From: Paul Eggert @ 2012-11-16  4:38 UTC (permalink / raw)
  To: 12881

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

Come to think of it, there's one more simplification:
remove the test for <fcntl.h> and the use of HAVE_FCNTL_H.
This should be safe since several modules already include <fcntl.h>
unconditionally.  Here's a patch, and I'll attach
a combined patch relative to trunk bzr 110908.

=== modified file 'ChangeLog'
--- ChangeLog	2012-11-16 01:35:18 +0000
+++ ChangeLog	2012-11-16 01:43:28 +0000
@@ -1,6 +1,7 @@
 2012-11-16  Paul Eggert  <eggert@cs.ucla.edu>
 
 	Assume POSIX 1003.1-1988 or later for fcntl.h (Bug#12881).
+	* configure.ac: Do not check for fcntl.h.
 	* lib/gnulib.mk: Regenerate.
 
 2012-11-14  Paul Eggert  <eggert@cs.ucla.edu>

=== modified file 'admin/CPP-DEFINES'
--- admin/CPP-DEFINES	2012-11-14 07:26:25 +0000
+++ admin/CPP-DEFINES	2012-11-16 04:33:15 +0000
@@ -150,7 +150,6 @@
 HAVE_ENDPWENT
 HAVE_ENVIRON_DECL
 HAVE_EUIDACCESS
-HAVE_FCNTL_H
 HAVE_FORK
 HAVE_FPATHCONF
 HAVE_FREEIFADDRS

=== modified file 'admin/ChangeLog'
--- admin/ChangeLog	2012-11-16 01:35:18 +0000
+++ admin/ChangeLog	2012-11-16 04:33:27 +0000
@@ -1,7 +1,7 @@
 2012-11-16  Paul Eggert  <eggert@cs.ucla.edu>
 
 	Assume POSIX 1003.1-1988 or later for fcntl.h (Bug#12881).
-	* CPP-DEFINES (O_RDONLY, O_RDWR): Remove.
+	* CPP-DEFINES (O_RDONLY, O_RDWR, HAVE_FCNTL_H): Remove.
 	* merge-gnulib (GNULIB_MODULES): Add fcntl-h.
 
 2012-11-14  Paul Eggert  <eggert@cs.ucla.edu>

=== modified file 'configure.ac'
--- configure.ac	2012-11-14 04:55:41 +0000
+++ configure.ac	2012-11-16 01:39:47 +0000
@@ -1268,7 +1268,7 @@
 dnl checks for header files
 AC_CHECK_HEADERS_ONCE(
   linux/version.h sys/systeminfo.h
-  fcntl.h coff.h pty.h
+  coff.h pty.h
   sys/vlimit.h sys/resource.h
   sys/utsname.h pwd.h utmp.h dirent.h util.h)
 

=== modified file 'lib-src/ChangeLog'
--- lib-src/ChangeLog	2012-10-26 07:40:51 +0000
+++ lib-src/ChangeLog	2012-11-16 01:44:21 +0000
@@ -1,3 +1,8 @@
+2012-11-16  Paul Eggert  <eggert@cs.ucla.edu>
+
+	Assume POSIX 1003.1-1988 or later for fcntl.h (Bug#12881).
+	* movemail.c, update-game-score.c: Assume <fcntl.h> exists.
+
 2012-10-26  Glenn Morris  <rgm@gnu.org>
 
 	* Makefile.in (uninstall): No INSTALLABLES live in archlibdir.

=== modified file 'lib-src/movemail.c'
--- lib-src/movemail.c	2012-08-10 07:07:07 +0000
+++ lib-src/movemail.c	2012-11-16 01:38:58 +0000
@@ -65,9 +65,7 @@
 
 #include <getopt.h>
 #include <unistd.h>
-#ifdef HAVE_FCNTL_H
 #include <fcntl.h>
-#endif
 #include <string.h>
 #include "syswait.h"
 #ifdef MAIL_USE_POP

=== modified file 'lib-src/update-game-score.c'
--- lib-src/update-game-score.c	2012-07-11 05:44:06 +0000
+++ lib-src/update-game-score.c	2012-11-16 01:39:03 +0000
@@ -42,9 +42,7 @@
 #include <time.h>
 #include <pwd.h>
 #include <ctype.h>
-#ifdef HAVE_FCNTL_H
 #include <fcntl.h>
-#endif
 #include <sys/stat.h>
 #include <getopt.h>
 

=== modified file 'src/ChangeLog'
--- src/ChangeLog	2012-11-16 01:35:18 +0000
+++ src/ChangeLog	2012-11-16 01:44:43 +0000
@@ -5,6 +5,7 @@
 	* emacs.c, term.c (O_RDWR): Remove.
 	* keyboard.c (tty_read_avail_input): Use O_NONBLOCK rather than
 	O_NDELAY, since O_NONBLOCK is the standard name for this flag.
+	* nsterm.m: Assume <fcntl.h> exists.
 	* process.c (NON_BLOCKING_CONNECT, allocate_pty, create_process)
 	(create_pty, Fmake_network_process, server_accept_connection)
 	(wait_reading_process_output, init_process_emacs):

=== modified file 'src/nsterm.m'
--- src/nsterm.m	2012-11-14 04:55:41 +0000
+++ src/nsterm.m	2012-11-16 01:39:22 +0000
@@ -30,6 +30,7 @@
    interpretation of even the system includes. */
 #include <config.h>
 
+#include <fcntl.h>
 #include <math.h>
 #include <sys/types.h>
 #include <time.h>
@@ -40,10 +41,6 @@
 #include <c-strcase.h>
 #include <ftoastr.h>
 
-#ifdef HAVE_FCNTL_H
-#include <fcntl.h>
-#endif
-
 #include "lisp.h"
 #include "blockinput.h"
 #include "sysselect.h"

[-- Attachment #2: fcntl.txt.gz --]
[-- Type: application/x-gzip, Size: 6354 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#12881: Assume at least POSIX.1-1988 for fcntl.h
  2012-11-15 20:07       ` Paul Eggert
@ 2012-11-16  9:49         ` Eli Zaretskii
  2012-11-16 14:37           ` Stefan Monnier
  2012-11-17 22:17           ` Paul Eggert
  0 siblings, 2 replies; 22+ messages in thread
From: Eli Zaretskii @ 2012-11-16  9:49 UTC (permalink / raw)
  To: Paul Eggert; +Cc: 12881

> Date: Thu, 15 Nov 2012 12:07:37 -0800
> From: Paul Eggert <eggert@cs.ucla.edu>
> CC: 12881@debbugs.gnu.org
> 
> On 11/15/12 09:42, Eli Zaretskii wrote:
> > Thanks, please hold on to it for a few days, until I or someone else
> > will have time to study it more closely.
> 
> I'll hold onto it for a bit, but there's no need for a close review
> process here.  The code works fine on POSIXish hosts and we've done a
> quick scan for plausible failures on Windows hosts and have come up
> dry.  Any problems that come up on Windows will likely be either
> easy-to-fix compilation issues, or further simplifications of the code
> that will not fix bugs.

Thanks for waiting.

The reason I prefer to review the patches such as this one is to
minimize downtime for several Windows users who track the development
trunk.  As you see below, our initial discussion notwithstanding, your
patches still break the Windows build in several places.  Experience
shows that if the Windows build is broken, it remains broken until I
get to fix it, with no one else stepping forward to do that.  It is
IMO bad mojo for several good people to depend on my scarce free time
or sleeping hours.

Here's the result of reviewing the combined patch you've posted in
http://debbugs.gnu.org/cgi/bugreport.cgi?bug=12881#20:

  @@ -4847,23 +4795,16 @@
		else if (nread == -1 && errno == EWOULDBLOCK)
		  ;
   #endif
  -	      /* ISC 4.1 defines both EWOULDBLOCK and O_NONBLOCK,
  -		 and Emacs uses O_NONBLOCK, so what we get is EAGAIN.  */
  -#if O_NONBLOCK
  -	      else if (nread == -1 && errno == EAGAIN)
  -		;
  -#else
  -#if O_NDELAY
  -	      else if (nread == -1 && errno == EAGAIN)
  -		;
  +	      else if (nread == -1 && errno == EAGAIN)
  +		;
  +#ifdef DOS_NT
		/* Note that we cannot distinguish between no input
		   available now and a closed pipe.
		   With luck, a closed pipe will be accompanied by
		   subprocess termination and SIGCHLD.  */
		else if (nread == 0 && !NETCONN_P (proc) && !SERIALCONN_P (proc))
		  ;
  -#endif /* O_NDELAY */
  -#endif /* O_NONBLOCK */
  +#endif

This hunk from process.c patches should use WINDOWSNT, not DOS_NT,
since MSDOS does not compile that part of process.c.

  @@ -1278,8 +1276,7 @@
     fsync (fileno (tty_out->output));
   #endif

  -#ifdef F_SETFL
  -#ifdef F_SETOWN		/* F_SETFL does not imply existence of F_SETOWN */
  +#ifdef F_SETOWN
     if (interrupt_input)
       {
	 reset_sigio (fileno (tty_out->input));
  @@ -1287,11 +1284,8 @@
		old_fcntl_owner[fileno (tty_out->input)]);
       }
   #endif /* F_SETOWN */
  -#if O_NDELAY
     fcntl (fileno (tty_out->input), F_SETFL,
  -         fcntl (fileno (tty_out->input), F_GETFL, 0) & ~O_NDELAY);
  -#endif
  -#endif /* F_SETFL */
  +         fcntl (fileno (tty_out->input), F_GETFL, 0) & ~O_NONBLOCK);

     if (tty_out->old_tty)
       while (emacs_set_tty (fileno (tty_out->input),

In this hunk, the block that was conditioned on F_SETFL should now be
conditioned on !DOS_NT.  The reason is that, although the Windows
build does have F_SETFL defined, it is defined in sys/socket.h, which
is not included by sysdep.c in the MS-Windows build.  So this block of
code was never compiled on Windows, and should continue not being
compiled there, since it invokes fcntl on TTY handles, something
that's not supported by w32.c:fcntl emulation, and also uses F_GETFL
which is not defined on Windows.

  --- src/term.c	2012-11-14 04:55:41 +0000
  +++ src/term.c	2012-11-14 07:26:25 +0000
  @@ -55,14 +55,6 @@
   #include "xterm.h"
   #endif

  -#ifndef O_RDWR
  -#define O_RDWR 2
  -#endif
  -
  -#ifndef O_NOCTTY
  -#define O_NOCTTY 0
  -#endif
  -
   /* The name of the default console device.  */
   #ifdef WINDOWSNT

This hunk will break MS-Windows compilation because O_NOCTTY is no
longer defined.  I suggest adding the definition in the WINDOWSNT
block just below, where the default console device name is defined.

Also, you need to include fcntl.h in term.c, to get both O_RDRW and
O_NOCTTY actually defined.  I wonder how it compiled for you;
presumably some other header on your platform includes fcntl.h
internally, but we shouldn't depend on that, IMO.

With those changes, I think the patch is safe to go into the trunk.
Thanks.






^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#12881: Assume at least POSIX.1-1988 for fcntl.h
  2012-11-16  9:49         ` Eli Zaretskii
@ 2012-11-16 14:37           ` Stefan Monnier
  2012-11-16 14:55             ` Juanma Barranquero
  2012-11-17 22:17           ` Paul Eggert
  1 sibling, 1 reply; 22+ messages in thread
From: Stefan Monnier @ 2012-11-16 14:37 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Paul Eggert, 12881

> The reason I prefer to review the patches such as this one is to
> minimize downtime for several Windows users who track the
> development trunk.

Actually, they should track `emacs-24' nowadays anyway ;-)


        Stefan





^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#12881: Assume at least POSIX.1-1988 for fcntl.h
  2012-11-16 14:37           ` Stefan Monnier
@ 2012-11-16 14:55             ` Juanma Barranquero
  2012-11-16 15:26               ` Stefan Monnier
  0 siblings, 1 reply; 22+ messages in thread
From: Juanma Barranquero @ 2012-11-16 14:55 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Paul Eggert, 12881

On Fri, Nov 16, 2012 at 3:37 PM, Stefan Monnier
<monnier@iro.umontreal.ca> wrote:

> Actually, they should track `emacs-24' nowadays anyway ;-)

At least one of them it is doing it now... ;-)

But as for Eli's comment, he's quite right in saying that, when things
break, he's the only one to fix them. Many of the recent changes that
have broken Windows need to be fixed by someone who's not only
well-versed on Windows development, but also POSIX. Unfortunately the
number of people with that skill set is tiny among Emacs developers, I
think.

    Juanma





^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#12881: Assume at least POSIX.1-1988 for fcntl.h
  2012-11-16 14:55             ` Juanma Barranquero
@ 2012-11-16 15:26               ` Stefan Monnier
  2012-11-16 16:00                 ` Eli Zaretskii
  0 siblings, 1 reply; 22+ messages in thread
From: Stefan Monnier @ 2012-11-16 15:26 UTC (permalink / raw)
  To: Juanma Barranquero; +Cc: Paul Eggert, 12881

> But as for Eli's comment, he's quite right in saying that, when things
> break, he's the only one to fix them.  Many of the recent changes that
> have broken Windows need to be fixed by someone who's not only
> well-versed on Windows development, but also POSIX.  Unfortunately the
> number of people with that skill set is tiny among Emacs developers,
> I think.

Yes, indeed.  I find it a bit scary, actually,


        Stefan





^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#12881: Assume at least POSIX.1-1988 for fcntl.h
  2012-11-16 15:26               ` Stefan Monnier
@ 2012-11-16 16:00                 ` Eli Zaretskii
  2012-11-16 16:29                   ` Juanma Barranquero
  2012-11-16 17:10                   ` Stefan Monnier
  0 siblings, 2 replies; 22+ messages in thread
From: Eli Zaretskii @ 2012-11-16 16:00 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: lekktu, eggert, 12881

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Eli Zaretskii <eliz@gnu.org>,  Paul Eggert <eggert@cs.ucla.edu>,  12881@debbugs.gnu.org
> Date: Fri, 16 Nov 2012 10:26:11 -0500
> 
> > But as for Eli's comment, he's quite right in saying that, when things
> > break, he's the only one to fix them.  Many of the recent changes that
> > have broken Windows need to be fixed by someone who's not only
> > well-versed on Windows development, but also POSIX.  Unfortunately the
> > number of people with that skill set is tiny among Emacs developers,
> > I think.
> 
> Yes, indeed.  I find it a bit scary, actually,

It's not.  It involves careful reading of the code and of the Posix
spec, that's all.





^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#12881: Assume at least POSIX.1-1988 for fcntl.h
  2012-11-16 16:00                 ` Eli Zaretskii
@ 2012-11-16 16:29                   ` Juanma Barranquero
  2012-11-16 17:41                     ` Eli Zaretskii
  2012-11-16 17:10                   ` Stefan Monnier
  1 sibling, 1 reply; 22+ messages in thread
From: Juanma Barranquero @ 2012-11-16 16:29 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: eggert, 12881

On Fri, Nov 16, 2012 at 5:00 PM, Eli Zaretskii <eliz@gnu.org> wrote:

> It's not.  It involves careful reading of the code and of the Posix
> spec, that's all.

Well, perhaps. But watching the back-and-forth between Paul and you,
it seems to me that things aren't so straightforward. A "careful
reading" of such a snake pit doesn't seem to me to be that far from my
"well versed" description.

    Juanma





^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#12881: Assume at least POSIX.1-1988 for fcntl.h
  2012-11-16 16:00                 ` Eli Zaretskii
  2012-11-16 16:29                   ` Juanma Barranquero
@ 2012-11-16 17:10                   ` Stefan Monnier
  2012-11-16 17:14                     ` Juanma Barranquero
  2012-11-16 17:44                     ` Eli Zaretskii
  1 sibling, 2 replies; 22+ messages in thread
From: Stefan Monnier @ 2012-11-16 17:10 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: lekktu, eggert, 12881

>> > But as for Eli's comment, he's quite right in saying that, when things
>> > break, he's the only one to fix them.  Many of the recent changes that
>> > have broken Windows need to be fixed by someone who's not only
>> > well-versed on Windows development, but also POSIX.  Unfortunately the
>> > number of people with that skill set is tiny among Emacs developers,
>> > I think.
>> Yes, indeed.  I find it a bit scary, actually,
> It's not.

I'm not sure we're talking about the same thing.  What scares me is the
"what would happen without Eli?".


        Stefan





^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#12881: Assume at least POSIX.1-1988 for fcntl.h
  2012-11-16 17:10                   ` Stefan Monnier
@ 2012-11-16 17:14                     ` Juanma Barranquero
  2012-11-16 17:20                       ` Juanma Barranquero
  2012-11-16 17:44                     ` Eli Zaretskii
  1 sibling, 1 reply; 22+ messages in thread
From: Juanma Barranquero @ 2012-11-16 17:14 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: eggert, 12881

On Fri, Nov 16, 2012 at 6:10 PM, Stefan Monnier
<monnier@iro.umontreal.ca> wrote:

> What scares me is the "what would happen without Eli?".

+1

    Juanma





^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#12881: Assume at least POSIX.1-1988 for fcntl.h
  2012-11-16 17:14                     ` Juanma Barranquero
@ 2012-11-16 17:20                       ` Juanma Barranquero
  0 siblings, 0 replies; 22+ messages in thread
From: Juanma Barranquero @ 2012-11-16 17:20 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: eggert, 12881

> On Fri, Nov 16, 2012 at 6:10 PM, Stefan Monnier

> What scares me is the "what would happen without Eli?".

Aside: the first thing that would happen (in about five minutes, ten
tops) is that someone would commit a change removing the MS-DOS
support and renaming a bunch of files to use long names...

(Sorry, couldn't resist)

    Juanma





^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#12881: Assume at least POSIX.1-1988 for fcntl.h
  2012-11-16 16:29                   ` Juanma Barranquero
@ 2012-11-16 17:41                     ` Eli Zaretskii
  0 siblings, 0 replies; 22+ messages in thread
From: Eli Zaretskii @ 2012-11-16 17:41 UTC (permalink / raw)
  To: Juanma Barranquero; +Cc: eggert, 12881

> From: Juanma Barranquero <lekktu@gmail.com>
> Date: Fri, 16 Nov 2012 17:29:30 +0100
> Cc: Stefan Monnier <monnier@iro.umontreal.ca>, eggert@cs.ucla.edu, 12881@debbugs.gnu.org
> 
> On Fri, Nov 16, 2012 at 5:00 PM, Eli Zaretskii <eliz@gnu.org> wrote:
> 
> > It's not.  It involves careful reading of the code and of the Posix
> > spec, that's all.
> 
> Well, perhaps. But watching the back-and-forth between Paul and you,
> it seems to me that things aren't so straightforward.

Most of that is just differences of opinions and styles, actually.
The factual basis is more or less known, solid, and not subject to
dispute.





^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#12881: Assume at least POSIX.1-1988 for fcntl.h
  2012-11-16 17:10                   ` Stefan Monnier
  2012-11-16 17:14                     ` Juanma Barranquero
@ 2012-11-16 17:44                     ` Eli Zaretskii
  2012-11-16 17:52                       ` Drew Adams
  1 sibling, 1 reply; 22+ messages in thread
From: Eli Zaretskii @ 2012-11-16 17:44 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: lekktu, eggert, 12881

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: lekktu@gmail.com,  eggert@cs.ucla.edu,  12881@debbugs.gnu.org
> Date: Fri, 16 Nov 2012 12:10:11 -0500
> 
> >> > But as for Eli's comment, he's quite right in saying that, when things
> >> > break, he's the only one to fix them.  Many of the recent changes that
> >> > have broken Windows need to be fixed by someone who's not only
> >> > well-versed on Windows development, but also POSIX.  Unfortunately the
> >> > number of people with that skill set is tiny among Emacs developers,
> >> > I think.
> >> Yes, indeed.  I find it a bit scary, actually,
> > It's not.
> 
> I'm not sure we're talking about the same thing.  What scares me is the
> "what would happen without Eli?".

That scares me as well.





^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#12881: Assume at least POSIX.1-1988 for fcntl.h
  2012-11-16 17:44                     ` Eli Zaretskii
@ 2012-11-16 17:52                       ` Drew Adams
  0 siblings, 0 replies; 22+ messages in thread
From: Drew Adams @ 2012-11-16 17:52 UTC (permalink / raw)
  To: 'Eli Zaretskii', 'Stefan Monnier'
  Cc: 'Juanma Barranquero', eggert, 12881

> > >> Yes, indeed.  I find it a bit scary, actually,
> > > It's not.
> > 
> > I'm not sure we're talking about the same thing.  What 
> > scares me is the "what would happen without Eli?".
> That scares me as well.

I wasn't going to say anything, but I realized what Stefan meant and was
thinking the same thing (+1).  And not just for the resulting loss of Windows
knowledge/support, obviously.  Knock on wood.






^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#12881: Assume at least POSIX.1-1988 for fcntl.h
  2012-11-16  9:49         ` Eli Zaretskii
  2012-11-16 14:37           ` Stefan Monnier
@ 2012-11-17 22:17           ` Paul Eggert
  2012-11-18  3:46             ` Eli Zaretskii
  1 sibling, 1 reply; 22+ messages in thread
From: Paul Eggert @ 2012-11-17 22:17 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 12881-done

On 11/16/2012 01:49 AM, Eli Zaretskii wrote:
> I wonder how it compiled for you;
> presumably some other header on your platform includes fcntl.h
> internally

It's systty.h, which includes fcntl.h everywhere.
But it doesn't hurt to include it again, so I did that.
I took the other suggestions you made as well, except
that I moved the replacement definitions of O_RDWR and
O_NOCTTY into nt/inc/unistd.h rather than to some other
spot in term.c.  It's cleaner to define system macros
in our implementation of the system rather than in the
application.  In the longer run we may want to move those
macros to a new file that replaces fcntl.h, but that's
for another day.

I installed this as trunk bzr 110931 and am marking it as
done.





^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#12881: Assume at least POSIX.1-1988 for fcntl.h
  2012-11-17 22:17           ` Paul Eggert
@ 2012-11-18  3:46             ` Eli Zaretskii
  2012-11-18  4:41               ` Paul Eggert
  0 siblings, 1 reply; 22+ messages in thread
From: Eli Zaretskii @ 2012-11-18  3:46 UTC (permalink / raw)
  To: Paul Eggert; +Cc: 12881

> Date: Sat, 17 Nov 2012 14:17:14 -0800
> From: Paul Eggert <eggert@cs.ucla.edu>
> CC: 12881-done@debbugs.gnu.org
> 
> I moved the replacement definitions of O_RDWR and
> O_NOCTTY into nt/inc/unistd.h rather than to some other
> spot in term.c.

I wish you'd discuss this first.  O_RDWR doesn't need to be defined
(it is already defined by a Windows system header), and defining
O_NOCTTY in a header included by many sources is wrong, because it
triggers compilation of other non-Windows friendly code elsewhere.

Looks like I invested all that effort in vain, and the Windows build
will be broken anyway.  Sigh.





^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#12881: Assume at least POSIX.1-1988 for fcntl.h
  2012-11-18  3:46             ` Eli Zaretskii
@ 2012-11-18  4:41               ` Paul Eggert
  2012-11-18 16:58                 ` Eli Zaretskii
  0 siblings, 1 reply; 22+ messages in thread
From: Paul Eggert @ 2012-11-18  4:41 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 12881

On 11/17/2012 07:46 PM, Eli Zaretskii wrote:
> O_RDWR doesn't need to be defined
> (it is already defined by a Windows system header)

OK, sorry, I misunderstood your earlier recommendation (to move
O_NOCTTY) to also refer to O_RDWR.  If you like, I can easily
remove the unnecessary definition of O_RDWR in nt/inc/unistd.h.

> defining O_NOCTTY in a header included by many sources is wrong, because it
> triggers compilation of other non-Windows friendly code elsewhere.
  
No, there's no "#ifdef O_NOCTTY" or "#if ... defined O_NOCTTY ..."
anywhere in Emacs that would be affected by this definition.
The non-Microsoft code already falls back on "#define O_NOCTTY 0"
for platforms that lack O_NOCTTY, and the Microsoft code can
safely do the same.

> Looks like I invested all that effort in vain

I don't see why here, either.  Neither of these appear to be problems
that would break a build or cause Emacs to misbehave.  And even if they
were, any fixes would be trivial.





^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#12881: Assume at least POSIX.1-1988 for fcntl.h
  2012-11-18  4:41               ` Paul Eggert
@ 2012-11-18 16:58                 ` Eli Zaretskii
  0 siblings, 0 replies; 22+ messages in thread
From: Eli Zaretskii @ 2012-11-18 16:58 UTC (permalink / raw)
  To: Paul Eggert; +Cc: 12881

> Date: Sat, 17 Nov 2012 20:41:24 -0800
> From: Paul Eggert <eggert@cs.ucla.edu>
> CC: 12881@debbugs.gnu.org
> 
> On 11/17/2012 07:46 PM, Eli Zaretskii wrote:
> > O_RDWR doesn't need to be defined
> > (it is already defined by a Windows system header)
> 
> OK, sorry, I misunderstood your earlier recommendation (to move
> O_NOCTTY) to also refer to O_RDWR.  If you like, I can easily
> remove the unnecessary definition of O_RDWR in nt/inc/unistd.h.

Thanks, I already did that.

> > Looks like I invested all that effort in vain
> 
> I don't see why here, either.  Neither of these appear to be problems
> that would break a build or cause Emacs to misbehave.  And even if they
> were, any fixes would be trivial.

I tried for once to make this commit clean, not requiring any fixes at
all.  I hope in the future, if you decide to do things that affect the
w32 build differently than what I suggest, we could discuss this
first, especially if I already invested some effort into making the
changes as clean as possible for Windows.  TIA.





^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2012-11-18 16:58 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-11-14  7:37 bug#12881: Assume at least POSIX.1-1988 for fcntl.h Paul Eggert
2012-11-14 17:33 ` Eli Zaretskii
2012-11-15  6:32   ` Paul Eggert
2012-11-15 17:42     ` Eli Zaretskii
2012-11-15 20:07       ` Paul Eggert
2012-11-16  9:49         ` Eli Zaretskii
2012-11-16 14:37           ` Stefan Monnier
2012-11-16 14:55             ` Juanma Barranquero
2012-11-16 15:26               ` Stefan Monnier
2012-11-16 16:00                 ` Eli Zaretskii
2012-11-16 16:29                   ` Juanma Barranquero
2012-11-16 17:41                     ` Eli Zaretskii
2012-11-16 17:10                   ` Stefan Monnier
2012-11-16 17:14                     ` Juanma Barranquero
2012-11-16 17:20                       ` Juanma Barranquero
2012-11-16 17:44                     ` Eli Zaretskii
2012-11-16 17:52                       ` Drew Adams
2012-11-17 22:17           ` Paul Eggert
2012-11-18  3:46             ` Eli Zaretskii
2012-11-18  4:41               ` Paul Eggert
2012-11-18 16:58                 ` Eli Zaretskii
2012-11-16  4:38 ` Paul Eggert

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

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).