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 <br...@clisp.org> 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