Re: [PATCH] execute: drop dead code

[Bruno Haible]

Hi Eric,

> The warning first surfaced when commit bdaf232 (Nov 2012) finally
> pointed out that these wrappers were no longer needed on posix-y
> systems, although the code has been unused since commit d629f6d
> (Jan 2009) which removed all use of open()/close() in favor of
> posix_spawn() instead.  The only platform remaining where the
> wrappers are used (and no warnings issued) is mingw, but according
> to Microsoft's documentation [1] at the time of this patch, mingw's
> libc never fails open or close with EINTR (not to mention that
> the documented purpose of the wrapper is for SIGSTOP, which mingw
> doesn't really have).
> 
> [1]http://msdn.microsoft.com/en-us/library/z0kc8e3z%28v=vs.80%29.aspx

OK for the argumentation for the POSIXy systems. But for the Windows/mingw
libc, I would be more careful. mingw is a moving target (it moves closer
towards POSIX over time), and has EINTR. It is not unreasonable to expect
that EINTR for open() or close() may occur in mingw at some point.

Btw, in my experience, in many documentation pages of Windows API and
Microsoft libc functions, there is at least one detail in the documentation
that does not match the actual behaviour. Therefore, just because the doc
doesn't mention EINTR as a possible return code, doesn't mean that EINTR
cannot occur as a return code.

> * lib/execute.c (nonintr_close, nonintr_open): Delete.

I would have changed the
  #ifdef EINTR
to
  #if defined EINTR && ((defined _WIN32 || defined __WIN32__) && ! defined 
__CYGWIN__)
similar to the approach taken in lib/spawn-pipe.c, which is a "brother" of 
lib/execute.c.

Are you OK with this partial revert?


2013-03-05  Bruno Haible  <address@hidden>

        execute: Revert last change, but use a different condition.
        * lib/execute.c (nonintr_close, nonintr_open): Reintroduce, but only on 
Windows.

--- lib/execute.c.orig  Wed Mar  6 02:07:38 2013
+++ lib/execute.c       Wed Mar  6 02:07:25 2013
@@ -54,6 +54,42 @@
 #undef close
 
 
+#if defined EINTR && ((defined _WIN32 || defined __WIN32__) && ! defined 
__CYGWIN__)
+
+/* EINTR handling for close(), open().
+   These functions can return -1/EINTR even though we don't have any
+   signal handlers set up, namely when we get interrupted via SIGSTOP.  */
+
+static int
+nonintr_close (int fd)
+{
+  int retval;
+
+  do
+    retval = close (fd);
+  while (retval < 0 && errno == EINTR);
+
+  return retval;
+}
+#define close nonintr_close
+
+static int
+nonintr_open (const char *pathname, int oflag, mode_t mode)
+{
+  int retval;
+
+  do
+    retval = open (pathname, oflag, mode);
+  while (retval < 0 && errno == EINTR);
+
+  return retval;
+}
+#undef open /* avoid warning on VMS */
+#define open nonintr_open
+
+#endif
+
+
 /* Execute a command, optionally redirecting any of the three standard file
    descriptors to /dev/null.  Return its exit code.
    If it didn't terminate correctly, exit if exit_on_error is true, otherwise