[PATCH v4 0/3] port upload-archive to Windows

[PATCH v4 0/3] port upload-archive to Windows

[Erik Faye-Lund]

Here's a new iteration of this series. I delayed it until the
improved version of "enter_repo: do not modify input" hit master,
which happened recently.

The important change in this iteration (besides the patch that
already propagated upstream) is that I've moved
compat/win32/sys/poll.[ch] out of the sys-folder (as XSI suggests).
This enables us to easily upgrade the poll-emulation without
breaking the Windows build in the process.

Erik Faye-Lund (3):
  mingw: move poll out of sys-folder
  compat/win32/poll.c: upgrade from upstream
  upload-archive: use start_command instead of fork

 Makefile                 |    6 +-
 builtin/archive.c        |    6 +-
 builtin/upload-archive.c |   68 ++----
 compat/mingw.h           |    2 -
 compat/win32/poll.c      |  606 ++++++++++++++++++++++++++++++++++++++++++++++
 compat/win32/poll.h      |   53 ++++
 compat/win32/sys/poll.c  |  599 ---------------------------------------------
 compat/win32/sys/poll.h  |   53 ----
 t/t5000-tar-tree.sh      |   10 +-
 9 files changed, 694 insertions(+), 709 deletions(-)
 create mode 100644 compat/win32/poll.c
 create mode 100644 compat/win32/poll.h
 delete mode 100644 compat/win32/sys/poll.c
 delete mode 100644 compat/win32/sys/poll.h

-- 
1.7.7.msysgit.1.1.g7b316

[PATCH v4 1/3] mingw: move poll out of sys-folder

[Erik Faye-Lund]

Both XSI and upstream Gnulib versions expects to find poll.h at
the root of some include path, not inside the sys-folder.

This helps us when upgrading Gnulib.

Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>
---
 Makefile                |    6 +-
 compat/win32/poll.c     |  599 +++++++++++++++++++++++++++++++++++++++++++++++
 compat/win32/poll.h     |   53 +++++
 compat/win32/sys/poll.c |  599 -----------------------------------------------
 compat/win32/sys/poll.h |   53 -----
 5 files changed, 656 insertions(+), 654 deletions(-)
 create mode 100644 compat/win32/poll.c
 create mode 100644 compat/win32/poll.h
 delete mode 100644 compat/win32/sys/poll.c
 delete mode 100644 compat/win32/sys/poll.h

diff --git a/Makefile b/Makefile
index 3139c19..4c09b35 100644
--- a/Makefile
+++ b/Makefile
@@ -1088,6 +1088,7 @@ ifeq ($(uname_S),Windows)
 	NO_PREAD = YesPlease
 	NEEDS_CRYPTO_WITH_SSL = YesPlease
 	NO_LIBGEN_H = YesPlease
+	NO_SYS_POLL_H = YesPlease
 	NO_SYMLINK_HEAD = YesPlease
 	NO_IPV6 = YesPlease
 	NO_SETENV = YesPlease
@@ -1126,7 +1127,7 @@ ifeq ($(uname_S),Windows)
 	BASIC_CFLAGS = -nologo -I. -I../zlib -Icompat/vcbuild -Icompat/vcbuild/include -DWIN32 -D_CONSOLE -DHAVE_STRING_H -D_CRT_SECURE_NO_WARNINGS -D_CRT_NONSTDC_NO_DEPRECATE
 	COMPAT_OBJS = compat/msvc.o compat/winansi.o \
 		compat/win32/pthread.o compat/win32/syslog.o \
-		compat/win32/sys/poll.o compat/win32/dirent.o
+		compat/win32/poll.o compat/win32/dirent.o
 	COMPAT_CFLAGS = -D__USE_MINGW_ACCESS -DNOGDI -DHAVE_STRING_H -DHAVE_ALLOCA_H -Icompat -Icompat/regex -Icompat/win32 -DSTRIP_EXTENSION=\".exe\"
 	BASIC_LDFLAGS = -IGNORE:4217 -IGNORE:4049 -NOLOGO -SUBSYSTEM:CONSOLE -NODEFAULTLIB:MSVCRT.lib
 	EXTLIBS = user32.lib advapi32.lib shell32.lib wininet.lib ws2_32.lib
@@ -1181,6 +1182,7 @@ ifneq (,$(findstring MINGW,$(uname_S)))
 	NO_PREAD = YesPlease
 	NEEDS_CRYPTO_WITH_SSL = YesPlease
 	NO_LIBGEN_H = YesPlease
+	NO_SYS_POLL_H = YesPlease
 	NO_SYMLINK_HEAD = YesPlease
 	NO_SETENV = YesPlease
 	NO_UNSETENV = YesPlease
@@ -1214,7 +1216,7 @@ ifneq (,$(findstring MINGW,$(uname_S)))
 	COMPAT_CFLAGS += -DSTRIP_EXTENSION=\".exe\"
 	COMPAT_OBJS += compat/mingw.o compat/winansi.o \
 		compat/win32/pthread.o compat/win32/syslog.o \
-		compat/win32/sys/poll.o compat/win32/dirent.o
+		compat/win32/poll.o compat/win32/dirent.o
 	EXTLIBS += -lws2_32
 	PTHREAD_LIBS =
 	X = .exe
diff --git a/compat/win32/poll.c b/compat/win32/poll.c
new file mode 100644
index 0000000..708a6c9
--- /dev/null
+++ b/compat/win32/poll.c
@@ -0,0 +1,599 @@
+/* Emulation for poll(2)
+   Contributed by Paolo Bonzini.
+
+   Copyright 2001-2003, 2006-2010 Free Software Foundation, Inc.
+
+   This file is part of gnulib.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 2, or (at your option)
+   any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License along
+   with this program; if not, write to the Free Software Foundation,
+   Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.  */
+
+/* Tell gcc not to warn about the (nfd < 0) tests, below.  */
+#if (__GNUC__ == 4 && 3 <= __GNUC_MINOR__) || 4 < __GNUC__
+# pragma GCC diagnostic ignored "-Wtype-limits"
+#endif
+
+#include <malloc.h>
+
+#include <sys/types.h>
+#include "poll.h"
+#include <errno.h>
+#include <limits.h>
+#include <assert.h>
+
+#if (defined _WIN32 || defined __WIN32__) && ! defined __CYGWIN__
+# define WIN32_NATIVE
+# if defined (_MSC_VER)
+#  define _WIN32_WINNT 0x0502
+# endif
+# include <winsock2.h>
+# include <windows.h>
+# include <io.h>
+# include <stdio.h>
+# include <conio.h>
+#else
+# include <sys/time.h>
+# include <sys/socket.h>
+# include <sys/select.h>
+# include <unistd.h>
+#endif
+
+#ifdef HAVE_SYS_IOCTL_H
+# include <sys/ioctl.h>
+#endif
+#ifdef HAVE_SYS_FILIO_H
+# include <sys/filio.h>
+#endif
+
+#include <time.h>
+
+#ifndef INFTIM
+# define INFTIM (-1)
+#endif
+
+/* BeOS does not have MSG_PEEK.  */
+#ifndef MSG_PEEK
+# define MSG_PEEK 0
+#endif
+
+#ifdef WIN32_NATIVE
+
+#define IsConsoleHandle(h) (((long) (h) & 3) == 3)
+
+static BOOL
+IsSocketHandle (HANDLE h)
+{
+  WSANETWORKEVENTS ev;
+
+  if (IsConsoleHandle (h))
+    return FALSE;
+
+  /* Under Wine, it seems that getsockopt returns 0 for pipes too.
+     WSAEnumNetworkEvents instead distinguishes the two correctly.  */
+  ev.lNetworkEvents = 0xDEADBEEF;
+  WSAEnumNetworkEvents ((SOCKET) h, NULL, &ev);
+  return ev.lNetworkEvents != 0xDEADBEEF;
+}
+
+/* Declare data structures for ntdll functions.  */
+typedef struct _FILE_PIPE_LOCAL_INFORMATION {
+  ULONG NamedPipeType;
+  ULONG NamedPipeConfiguration;
+  ULONG MaximumInstances;
+  ULONG CurrentInstances;
+  ULONG InboundQuota;
+  ULONG ReadDataAvailable;
+  ULONG OutboundQuota;
+  ULONG WriteQuotaAvailable;
+  ULONG NamedPipeState;
+  ULONG NamedPipeEnd;
+} FILE_PIPE_LOCAL_INFORMATION, *PFILE_PIPE_LOCAL_INFORMATION;
+
+typedef struct _IO_STATUS_BLOCK
+{
+  union {
+    DWORD Status;
+    PVOID Pointer;
+  } u;
+  ULONG_PTR Information;
+} IO_STATUS_BLOCK, *PIO_STATUS_BLOCK;
+
+typedef enum _FILE_INFORMATION_CLASS {
+  FilePipeLocalInformation = 24
+} FILE_INFORMATION_CLASS, *PFILE_INFORMATION_CLASS;
+
+typedef DWORD (WINAPI *PNtQueryInformationFile)
+	 (HANDLE, IO_STATUS_BLOCK *, VOID *, ULONG, FILE_INFORMATION_CLASS);
+
+# ifndef PIPE_BUF
+#  define PIPE_BUF      512
+# endif
+
+/* Compute revents values for file handle H.  If some events cannot happen
+   for the handle, eliminate them from *P_SOUGHT.  */
+
+static int
+win32_compute_revents (HANDLE h, int *p_sought)
+{
+  int i, ret, happened;
+  INPUT_RECORD *irbuffer;
+  DWORD avail, nbuffer;
+  BOOL bRet;
+  IO_STATUS_BLOCK iosb;
+  FILE_PIPE_LOCAL_INFORMATION fpli;
+  static PNtQueryInformationFile NtQueryInformationFile;
+  static BOOL once_only;
+
+  switch (GetFileType (h))
+    {
+    case FILE_TYPE_PIPE:
+      if (!once_only)
+	{
+	  NtQueryInformationFile = (PNtQueryInformationFile)
+	    GetProcAddress (GetModuleHandle ("ntdll.dll"),
+			    "NtQueryInformationFile");
+	  once_only = TRUE;
+	}
+
+      happened = 0;
+      if (PeekNamedPipe (h, NULL, 0, NULL, &avail, NULL) != 0)
+	{
+	  if (avail)
+	    happened |= *p_sought & (POLLIN | POLLRDNORM);
+	}
+      else if (GetLastError () == ERROR_BROKEN_PIPE)
+	happened |= POLLHUP;
+
+      else
+	{
+	  /* It was the write-end of the pipe.  Check if it is writable.
+	     If NtQueryInformationFile fails, optimistically assume the pipe is
+	     writable.  This could happen on Win9x, where NtQueryInformationFile
+	     is not available, or if we inherit a pipe that doesn't permit
+	     FILE_READ_ATTRIBUTES access on the write end (I think this should
+	     not happen since WinXP SP2; WINE seems fine too).  Otherwise,
+	     ensure that enough space is available for atomic writes.  */
+	  memset (&iosb, 0, sizeof (iosb));
+	  memset (&fpli, 0, sizeof (fpli));
+
+	  if (!NtQueryInformationFile
+	      || NtQueryInformationFile (h, &iosb, &fpli, sizeof (fpli),
+					 FilePipeLocalInformation)
+	      || fpli.WriteQuotaAvailable >= PIPE_BUF
+	      || (fpli.OutboundQuota < PIPE_BUF &&
+		  fpli.WriteQuotaAvailable == fpli.OutboundQuota))
+	    happened |= *p_sought & (POLLOUT | POLLWRNORM | POLLWRBAND);
+	}
+      return happened;
+
+    case FILE_TYPE_CHAR:
+      ret = WaitForSingleObject (h, 0);
+      if (!IsConsoleHandle (h))
+	return ret == WAIT_OBJECT_0 ? *p_sought & ~(POLLPRI | POLLRDBAND) : 0;
+
+      nbuffer = avail = 0;
+      bRet = GetNumberOfConsoleInputEvents (h, &nbuffer);
+      if (bRet)
+	{
+	  /* Input buffer.  */
+	  *p_sought &= POLLIN | POLLRDNORM;
+	  if (nbuffer == 0)
+	    return POLLHUP;
+	  if (!*p_sought)
+	    return 0;
+
+	  irbuffer = (INPUT_RECORD *) alloca (nbuffer * sizeof (INPUT_RECORD));
+	  bRet = PeekConsoleInput (h, irbuffer, nbuffer, &avail);
+	  if (!bRet || avail == 0)
+	    return POLLHUP;
+
+	  for (i = 0; i < avail; i++)
+	    if (irbuffer[i].EventType == KEY_EVENT)
+	      return *p_sought;
+	  return 0;
+	}
+      else
+	{
+	  /* Screen buffer.  */
+	  *p_sought &= POLLOUT | POLLWRNORM | POLLWRBAND;
+	  return *p_sought;
+	}
+
+    default:
+      ret = WaitForSingleObject (h, 0);
+      if (ret == WAIT_OBJECT_0)
+	return *p_sought & ~(POLLPRI | POLLRDBAND);
+
+      return *p_sought & (POLLOUT | POLLWRNORM | POLLWRBAND);
+    }
+}
+
+/* Convert fd_sets returned by select into revents values.  */
+
+static int
+win32_compute_revents_socket (SOCKET h, int sought, long lNetworkEvents)
+{
+  int happened = 0;
+
+  if ((lNetworkEvents & (FD_READ | FD_ACCEPT | FD_CLOSE)) == FD_ACCEPT)
+    happened |= (POLLIN | POLLRDNORM) & sought;
+
+  else if (lNetworkEvents & (FD_READ | FD_ACCEPT | FD_CLOSE))
+    {
+      int r, error;
+
+      char data[64];
+      WSASetLastError (0);
+      r = recv (h, data, sizeof (data), MSG_PEEK);
+      error = WSAGetLastError ();
+      WSASetLastError (0);
+
+      if (r > 0 || error == WSAENOTCONN)
+	happened |= (POLLIN | POLLRDNORM) & sought;
+
+      /* Distinguish hung-up sockets from other errors.  */
+      else if (r == 0 || error == WSAESHUTDOWN || error == WSAECONNRESET
+	       || error == WSAECONNABORTED || error == WSAENETRESET)
+	happened |= POLLHUP;
+
+      else
+	happened |= POLLERR;
+    }
+
+  if (lNetworkEvents & (FD_WRITE | FD_CONNECT))
+    happened |= (POLLOUT | POLLWRNORM | POLLWRBAND) & sought;
+
+  if (lNetworkEvents & FD_OOB)
+    happened |= (POLLPRI | POLLRDBAND) & sought;
+
+  return happened;
+}
+
+#else /* !MinGW */
+
+/* Convert select(2) returned fd_sets into poll(2) revents values.  */
+static int
+compute_revents (int fd, int sought, fd_set *rfds, fd_set *wfds, fd_set *efds)
+{
+  int happened = 0;
+  if (FD_ISSET (fd, rfds))
+    {
+      int r;
+      int socket_errno;
+
+# if defined __MACH__ && defined __APPLE__
+      /* There is a bug in Mac OS X that causes it to ignore MSG_PEEK
+	 for some kinds of descriptors.  Detect if this descriptor is a
+	 connected socket, a server socket, or something else using a
+	 0-byte recv, and use ioctl(2) to detect POLLHUP.  */
+      r = recv (fd, NULL, 0, MSG_PEEK);
+      socket_errno = (r < 0) ? errno : 0;
+      if (r == 0 || socket_errno == ENOTSOCK)
+	ioctl (fd, FIONREAD, &r);
+# else
+      char data[64];
+      r = recv (fd, data, sizeof (data), MSG_PEEK);
+      socket_errno = (r < 0) ? errno : 0;
+# endif
+      if (r == 0)
+	happened |= POLLHUP;
+
+      /* If the event happened on an unconnected server socket,
+	 that's fine. */
+      else if (r > 0 || ( /* (r == -1) && */ socket_errno == ENOTCONN))
+	happened |= (POLLIN | POLLRDNORM) & sought;
+
+      /* Distinguish hung-up sockets from other errors.  */
+      else if (socket_errno == ESHUTDOWN || socket_errno == ECONNRESET
+	       || socket_errno == ECONNABORTED || socket_errno == ENETRESET)
+	happened |= POLLHUP;
+
+      else
+	happened |= POLLERR;
+    }
+
+  if (FD_ISSET (fd, wfds))
+    happened |= (POLLOUT | POLLWRNORM | POLLWRBAND) & sought;
+
+  if (FD_ISSET (fd, efds))
+    happened |= (POLLPRI | POLLRDBAND) & sought;
+
+  return happened;
+}
+#endif /* !MinGW */
+
+int
+poll (pfd, nfd, timeout)
+     struct pollfd *pfd;
+     nfds_t nfd;
+     int timeout;
+{
+#ifndef WIN32_NATIVE
+  fd_set rfds, wfds, efds;
+  struct timeval tv;
+  struct timeval *ptv;
+  int maxfd, rc;
+  nfds_t i;
+
+# ifdef _SC_OPEN_MAX
+  static int sc_open_max = -1;
+
+  if (nfd < 0
+      || (nfd > sc_open_max
+	  && (sc_open_max != -1
+	      || nfd > (sc_open_max = sysconf (_SC_OPEN_MAX)))))
+    {
+      errno = EINVAL;
+      return -1;
+    }
+# else /* !_SC_OPEN_MAX */
+#  ifdef OPEN_MAX
+  if (nfd < 0 || nfd > OPEN_MAX)
+    {
+      errno = EINVAL;
+      return -1;
+    }
+#  endif /* OPEN_MAX -- else, no check is needed */
+# endif /* !_SC_OPEN_MAX */
+
+  /* EFAULT is not necessary to implement, but let's do it in the
+     simplest case. */
+  if (!pfd)
+    {
+      errno = EFAULT;
+      return -1;
+    }
+
+  /* convert timeout number into a timeval structure */
+  if (timeout == 0)
+    {
+      ptv = &tv;
+      ptv->tv_sec = 0;
+      ptv->tv_usec = 0;
+    }
+  else if (timeout > 0)
+    {
+      ptv = &tv;
+      ptv->tv_sec = timeout / 1000;
+      ptv->tv_usec = (timeout % 1000) * 1000;
+    }
+  else if (timeout == INFTIM)
+    /* wait forever */
+    ptv = NULL;
+  else
+    {
+      errno = EINVAL;
+      return -1;
+    }
+
+  /* create fd sets and determine max fd */
+  maxfd = -1;
+  FD_ZERO (&rfds);
+  FD_ZERO (&wfds);
+  FD_ZERO (&efds);
+  for (i = 0; i < nfd; i++)
+    {
+      if (pfd[i].fd < 0)
+	continue;
+
+      if (pfd[i].events & (POLLIN | POLLRDNORM))
+	FD_SET (pfd[i].fd, &rfds);
+
+      /* see select(2): "the only exceptional condition detectable
+	 is out-of-band data received on a socket", hence we push
+	 POLLWRBAND events onto wfds instead of efds. */
+      if (pfd[i].events & (POLLOUT | POLLWRNORM | POLLWRBAND))
+	FD_SET (pfd[i].fd, &wfds);
+      if (pfd[i].events & (POLLPRI | POLLRDBAND))
+	FD_SET (pfd[i].fd, &efds);
+      if (pfd[i].fd >= maxfd
+	  && (pfd[i].events & (POLLIN | POLLOUT | POLLPRI
+			       | POLLRDNORM | POLLRDBAND
+			       | POLLWRNORM | POLLWRBAND)))
+	{
+	  maxfd = pfd[i].fd;
+	  if (maxfd > FD_SETSIZE)
+	    {
+	      errno = EOVERFLOW;
+	      return -1;
+	    }
+	}
+    }
+
+  /* examine fd sets */
+  rc = select (maxfd + 1, &rfds, &wfds, &efds, ptv);
+  if (rc < 0)
+    return rc;
+
+  /* establish results */
+  rc = 0;
+  for (i = 0; i < nfd; i++)
+    if (pfd[i].fd < 0)
+      pfd[i].revents = 0;
+    else
+      {
+	int happened = compute_revents (pfd[i].fd, pfd[i].events,
+					&rfds, &wfds, &efds);
+	if (happened)
+	  {
+	    pfd[i].revents = happened;
+	    rc++;
+	  }
+      }
+
+  return rc;
+#else
+  static struct timeval tv0;
+  static HANDLE hEvent;
+  WSANETWORKEVENTS ev;
+  HANDLE h, handle_array[FD_SETSIZE + 2];
+  DWORD ret, wait_timeout, nhandles;
+  fd_set rfds, wfds, xfds;
+  BOOL poll_again;
+  MSG msg;
+  int rc = 0;
+  nfds_t i;
+
+  if (nfd < 0 || timeout < -1)
+    {
+      errno = EINVAL;
+      return -1;
+    }
+
+  if (!hEvent)
+    hEvent = CreateEvent (NULL, FALSE, FALSE, NULL);
+
+  handle_array[0] = hEvent;
+  nhandles = 1;
+  FD_ZERO (&rfds);
+  FD_ZERO (&wfds);
+  FD_ZERO (&xfds);
+
+  /* Classify socket handles and create fd sets. */
+  for (i = 0; i < nfd; i++)
+    {
+      int sought = pfd[i].events;
+      pfd[i].revents = 0;
+      if (pfd[i].fd < 0)
+	continue;
+      if (!(sought & (POLLIN | POLLRDNORM | POLLOUT | POLLWRNORM | POLLWRBAND
+		      | POLLPRI | POLLRDBAND)))
+	continue;
+
+      h = (HANDLE) _get_osfhandle (pfd[i].fd);
+      assert (h != NULL);
+      if (IsSocketHandle (h))
+	{
+	  int requested = FD_CLOSE;
+
+	  /* see above; socket handles are mapped onto select.  */
+	  if (sought & (POLLIN | POLLRDNORM))
+	    {
+	      requested |= FD_READ | FD_ACCEPT;
+	      FD_SET ((SOCKET) h, &rfds);
+	    }
+	  if (sought & (POLLOUT | POLLWRNORM | POLLWRBAND))
+	    {
+	      requested |= FD_WRITE | FD_CONNECT;
+	      FD_SET ((SOCKET) h, &wfds);
+	    }
+	  if (sought & (POLLPRI | POLLRDBAND))
+	    {
+	      requested |= FD_OOB;
+	      FD_SET ((SOCKET) h, &xfds);
+	    }
+
+	  if (requested)
+	    WSAEventSelect ((SOCKET) h, hEvent, requested);
+	}
+      else
+	{
+	  /* Poll now.  If we get an event, do not poll again.  Also,
+	     screen buffer handles are waitable, and they'll block until
+	     a character is available.  win32_compute_revents eliminates
+	     bits for the "wrong" direction. */
+	  pfd[i].revents = win32_compute_revents (h, &sought);
+	  if (sought)
+	    handle_array[nhandles++] = h;
+	  if (pfd[i].revents)
+	    timeout = 0;
+	}
+    }
+
+  if (select (0, &rfds, &wfds, &xfds, &tv0) > 0)
+    {
+      /* Do MsgWaitForMultipleObjects anyway to dispatch messages, but
+	 no need to call select again.  */
+      poll_again = FALSE;
+      wait_timeout = 0;
+    }
+  else
+    {
+      poll_again = TRUE;
+      if (timeout == INFTIM)
+	wait_timeout = INFINITE;
+      else
+	wait_timeout = timeout;
+    }
+
+  for (;;)
+    {
+      ret = MsgWaitForMultipleObjects (nhandles, handle_array, FALSE,
+				       wait_timeout, QS_ALLINPUT);
+
+      if (ret == WAIT_OBJECT_0 + nhandles)
+	{
+	  /* new input of some other kind */
+	  BOOL bRet;
+	  while ((bRet = PeekMessage (&msg, NULL, 0, 0, PM_REMOVE)) != 0)
+	    {
+	      TranslateMessage (&msg);
+	      DispatchMessage (&msg);
+	    }
+	}
+      else
+	break;
+    }
+
+  if (poll_again)
+    select (0, &rfds, &wfds, &xfds, &tv0);
+
+  /* Place a sentinel at the end of the array.  */
+  handle_array[nhandles] = NULL;
+  nhandles = 1;
+  for (i = 0; i < nfd; i++)
+    {
+      int happened;
+
+      if (pfd[i].fd < 0)
+	continue;
+      if (!(pfd[i].events & (POLLIN | POLLRDNORM |
+			     POLLOUT | POLLWRNORM | POLLWRBAND)))
+	continue;
+
+      h = (HANDLE) _get_osfhandle (pfd[i].fd);
+      if (h != handle_array[nhandles])
+	{
+	  /* It's a socket.  */
+	  WSAEnumNetworkEvents ((SOCKET) h, NULL, &ev);
+	  WSAEventSelect ((SOCKET) h, 0, 0);
+
+	  /* If we're lucky, WSAEnumNetworkEvents already provided a way
+	     to distinguish FD_READ and FD_ACCEPT; this saves a recv later.  */
+	  if (FD_ISSET ((SOCKET) h, &rfds)
+	      && !(ev.lNetworkEvents & (FD_READ | FD_ACCEPT)))
+	    ev.lNetworkEvents |= FD_READ | FD_ACCEPT;
+	  if (FD_ISSET ((SOCKET) h, &wfds))
+	    ev.lNetworkEvents |= FD_WRITE | FD_CONNECT;
+	  if (FD_ISSET ((SOCKET) h, &xfds))
+	    ev.lNetworkEvents |= FD_OOB;
+
+	  happened = win32_compute_revents_socket ((SOCKET) h, pfd[i].events,
+						   ev.lNetworkEvents);
+	}
+      else
+	{
+	  /* Not a socket.  */
+	  int sought = pfd[i].events;
+	  happened = win32_compute_revents (h, &sought);
+	  nhandles++;
+	}
+
+       if ((pfd[i].revents |= happened) != 0)
+	rc++;
+    }
+
+  return rc;
+#endif
+}
diff --git a/compat/win32/poll.h b/compat/win32/poll.h
new file mode 100644
index 0000000..b7aa59d
--- /dev/null
+++ b/compat/win32/poll.h
@@ -0,0 +1,53 @@
+/* Header for poll(2) emulation
+   Contributed by Paolo Bonzini.
+
+   Copyright 2001, 2002, 2003, 2007, 2009, 2010 Free Software Foundation, Inc.
+
+   This file is part of gnulib.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 2, or (at your option)
+   any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License along
+   with this program; if not, write to the Free Software Foundation,
+   Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.  */
+
+#ifndef _GL_POLL_H
+#define _GL_POLL_H
+
+/* fake a poll(2) environment */
+#define POLLIN      0x0001      /* any readable data available   */
+#define POLLPRI     0x0002      /* OOB/Urgent readable data      */
+#define POLLOUT     0x0004      /* file descriptor is writeable  */
+#define POLLERR     0x0008      /* some poll error occurred      */
+#define POLLHUP     0x0010      /* file descriptor was "hung up" */
+#define POLLNVAL    0x0020      /* requested events "invalid"    */
+#define POLLRDNORM  0x0040
+#define POLLRDBAND  0x0080
+#define POLLWRNORM  0x0100
+#define POLLWRBAND  0x0200
+
+struct pollfd
+{
+  int fd;                       /* which file descriptor to poll */
+  short events;                 /* events we are interested in   */
+  short revents;                /* events found on return        */
+};
+
+typedef unsigned long nfds_t;
+
+extern int poll (struct pollfd *pfd, nfds_t nfd, int timeout);
+
+/* Define INFTIM only if doing so conforms to POSIX.  */
+#if !defined (_POSIX_C_SOURCE) && !defined (_XOPEN_SOURCE)
+#define INFTIM (-1)
+#endif
+
+#endif /* _GL_POLL_H */
diff --git a/compat/win32/sys/poll.c b/compat/win32/sys/poll.c
deleted file mode 100644
index 708a6c9..0000000
--- a/compat/win32/sys/poll.c
+++ /dev/null
@@ -1,599 +0,0 @@
-/* Emulation for poll(2)
-   Contributed by Paolo Bonzini.
-
-   Copyright 2001-2003, 2006-2010 Free Software Foundation, Inc.
-
-   This file is part of gnulib.
-
-   This program is free software; you can redistribute it and/or modify
-   it under the terms of the GNU General Public License as published by
-   the Free Software Foundation; either version 2, or (at your option)
-   any later version.
-
-   This program is distributed in the hope that it will be useful,
-   but WITHOUT ANY WARRANTY; without even the implied warranty of
-   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-   GNU General Public License for more details.
-
-   You should have received a copy of the GNU General Public License along
-   with this program; if not, write to the Free Software Foundation,
-   Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.  */
-
-/* Tell gcc not to warn about the (nfd < 0) tests, below.  */
-#if (__GNUC__ == 4 && 3 <= __GNUC_MINOR__) || 4 < __GNUC__
-# pragma GCC diagnostic ignored "-Wtype-limits"
-#endif
-
-#include <malloc.h>
-
-#include <sys/types.h>
-#include "poll.h"
-#include <errno.h>
-#include <limits.h>
-#include <assert.h>
-
-#if (defined _WIN32 || defined __WIN32__) && ! defined __CYGWIN__
-# define WIN32_NATIVE
-# if defined (_MSC_VER)
-#  define _WIN32_WINNT 0x0502
-# endif
-# include <winsock2.h>
-# include <windows.h>
-# include <io.h>
-# include <stdio.h>
-# include <conio.h>
-#else
-# include <sys/time.h>
-# include <sys/socket.h>
-# include <sys/select.h>
-# include <unistd.h>
-#endif
-
-#ifdef HAVE_SYS_IOCTL_H
-# include <sys/ioctl.h>
-#endif
-#ifdef HAVE_SYS_FILIO_H
-# include <sys/filio.h>
-#endif
-
-#include <time.h>
-
-#ifndef INFTIM
-# define INFTIM (-1)
-#endif
-
-/* BeOS does not have MSG_PEEK.  */
-#ifndef MSG_PEEK
-# define MSG_PEEK 0
-#endif
-
-#ifdef WIN32_NATIVE
-
-#define IsConsoleHandle(h) (((long) (h) & 3) == 3)
-
-static BOOL
-IsSocketHandle (HANDLE h)
-{
-  WSANETWORKEVENTS ev;
-
-  if (IsConsoleHandle (h))
-    return FALSE;
-
-  /* Under Wine, it seems that getsockopt returns 0 for pipes too.
-     WSAEnumNetworkEvents instead distinguishes the two correctly.  */
-  ev.lNetworkEvents = 0xDEADBEEF;
-  WSAEnumNetworkEvents ((SOCKET) h, NULL, &ev);
-  return ev.lNetworkEvents != 0xDEADBEEF;
-}
-
-/* Declare data structures for ntdll functions.  */
-typedef struct _FILE_PIPE_LOCAL_INFORMATION {
-  ULONG NamedPipeType;
-  ULONG NamedPipeConfiguration;
-  ULONG MaximumInstances;
-  ULONG CurrentInstances;
-  ULONG InboundQuota;
-  ULONG ReadDataAvailable;
-  ULONG OutboundQuota;
-  ULONG WriteQuotaAvailable;
-  ULONG NamedPipeState;
-  ULONG NamedPipeEnd;
-} FILE_PIPE_LOCAL_INFORMATION, *PFILE_PIPE_LOCAL_INFORMATION;
-
-typedef struct _IO_STATUS_BLOCK
-{
-  union {
-    DWORD Status;
-    PVOID Pointer;
-  } u;
-  ULONG_PTR Information;
-} IO_STATUS_BLOCK, *PIO_STATUS_BLOCK;
-
-typedef enum _FILE_INFORMATION_CLASS {
-  FilePipeLocalInformation = 24
-} FILE_INFORMATION_CLASS, *PFILE_INFORMATION_CLASS;
-
-typedef DWORD (WINAPI *PNtQueryInformationFile)
-	 (HANDLE, IO_STATUS_BLOCK *, VOID *, ULONG, FILE_INFORMATION_CLASS);
-
-# ifndef PIPE_BUF
-#  define PIPE_BUF      512
-# endif
-
-/* Compute revents values for file handle H.  If some events cannot happen
-   for the handle, eliminate them from *P_SOUGHT.  */
-
-static int
-win32_compute_revents (HANDLE h, int *p_sought)
-{
-  int i, ret, happened;
-  INPUT_RECORD *irbuffer;
-  DWORD avail, nbuffer;
-  BOOL bRet;
-  IO_STATUS_BLOCK iosb;
-  FILE_PIPE_LOCAL_INFORMATION fpli;
-  static PNtQueryInformationFile NtQueryInformationFile;
-  static BOOL once_only;
-
-  switch (GetFileType (h))
-    {
-    case FILE_TYPE_PIPE:
-      if (!once_only)
-	{
-	  NtQueryInformationFile = (PNtQueryInformationFile)
-	    GetProcAddress (GetModuleHandle ("ntdll.dll"),
-			    "NtQueryInformationFile");
-	  once_only = TRUE;
-	}
-
-      happened = 0;
-      if (PeekNamedPipe (h, NULL, 0, NULL, &avail, NULL) != 0)
-	{
-	  if (avail)
-	    happened |= *p_sought & (POLLIN | POLLRDNORM);
-	}
-      else if (GetLastError () == ERROR_BROKEN_PIPE)
-	happened |= POLLHUP;
-
-      else
-	{
-	  /* It was the write-end of the pipe.  Check if it is writable.
-	     If NtQueryInformationFile fails, optimistically assume the pipe is
-	     writable.  This could happen on Win9x, where NtQueryInformationFile
-	     is not available, or if we inherit a pipe that doesn't permit
-	     FILE_READ_ATTRIBUTES access on the write end (I think this should
-	     not happen since WinXP SP2; WINE seems fine too).  Otherwise,
-	     ensure that enough space is available for atomic writes.  */
-	  memset (&iosb, 0, sizeof (iosb));
-	  memset (&fpli, 0, sizeof (fpli));
-
-	  if (!NtQueryInformationFile
-	      || NtQueryInformationFile (h, &iosb, &fpli, sizeof (fpli),
-					 FilePipeLocalInformation)
-	      || fpli.WriteQuotaAvailable >= PIPE_BUF
-	      || (fpli.OutboundQuota < PIPE_BUF &&
-		  fpli.WriteQuotaAvailable == fpli.OutboundQuota))
-	    happened |= *p_sought & (POLLOUT | POLLWRNORM | POLLWRBAND);
-	}
-      return happened;
-
-    case FILE_TYPE_CHAR:
-      ret = WaitForSingleObject (h, 0);
-      if (!IsConsoleHandle (h))
-	return ret == WAIT_OBJECT_0 ? *p_sought & ~(POLLPRI | POLLRDBAND) : 0;
-
-      nbuffer = avail = 0;
-      bRet = GetNumberOfConsoleInputEvents (h, &nbuffer);
-      if (bRet)
-	{
-	  /* Input buffer.  */
-	  *p_sought &= POLLIN | POLLRDNORM;
-	  if (nbuffer == 0)
-	    return POLLHUP;
-	  if (!*p_sought)
-	    return 0;
-
-	  irbuffer = (INPUT_RECORD *) alloca (nbuffer * sizeof (INPUT_RECORD));
-	  bRet = PeekConsoleInput (h, irbuffer, nbuffer, &avail);
-	  if (!bRet || avail == 0)
-	    return POLLHUP;
-
-	  for (i = 0; i < avail; i++)
-	    if (irbuffer[i].EventType == KEY_EVENT)
-	      return *p_sought;
-	  return 0;
-	}
-      else
-	{
-	  /* Screen buffer.  */
-	  *p_sought &= POLLOUT | POLLWRNORM | POLLWRBAND;
-	  return *p_sought;
-	}
-
-    default:
-      ret = WaitForSingleObject (h, 0);
-      if (ret == WAIT_OBJECT_0)
-	return *p_sought & ~(POLLPRI | POLLRDBAND);
-
-      return *p_sought & (POLLOUT | POLLWRNORM | POLLWRBAND);
-    }
-}
-
-/* Convert fd_sets returned by select into revents values.  */
-
-static int
-win32_compute_revents_socket (SOCKET h, int sought, long lNetworkEvents)
-{
-  int happened = 0;
-
-  if ((lNetworkEvents & (FD_READ | FD_ACCEPT | FD_CLOSE)) == FD_ACCEPT)
-    happened |= (POLLIN | POLLRDNORM) & sought;
-
-  else if (lNetworkEvents & (FD_READ | FD_ACCEPT | FD_CLOSE))
-    {
-      int r, error;
-
-      char data[64];
-      WSASetLastError (0);
-      r = recv (h, data, sizeof (data), MSG_PEEK);
-      error = WSAGetLastError ();
-      WSASetLastError (0);
-
-      if (r > 0 || error == WSAENOTCONN)
-	happened |= (POLLIN | POLLRDNORM) & sought;
-
-      /* Distinguish hung-up sockets from other errors.  */
-      else if (r == 0 || error == WSAESHUTDOWN || error == WSAECONNRESET
-	       || error == WSAECONNABORTED || error == WSAENETRESET)
-	happened |= POLLHUP;
-
-      else
-	happened |= POLLERR;
-    }
-
-  if (lNetworkEvents & (FD_WRITE | FD_CONNECT))
-    happened |= (POLLOUT | POLLWRNORM | POLLWRBAND) & sought;
-
-  if (lNetworkEvents & FD_OOB)
-    happened |= (POLLPRI | POLLRDBAND) & sought;
-
-  return happened;
-}
-
-#else /* !MinGW */
-
-/* Convert select(2) returned fd_sets into poll(2) revents values.  */
-static int
-compute_revents (int fd, int sought, fd_set *rfds, fd_set *wfds, fd_set *efds)
-{
-  int happened = 0;
-  if (FD_ISSET (fd, rfds))
-    {
-      int r;
-      int socket_errno;
-
-# if defined __MACH__ && defined __APPLE__
-      /* There is a bug in Mac OS X that causes it to ignore MSG_PEEK
-	 for some kinds of descriptors.  Detect if this descriptor is a
-	 connected socket, a server socket, or something else using a
-	 0-byte recv, and use ioctl(2) to detect POLLHUP.  */
-      r = recv (fd, NULL, 0, MSG_PEEK);
-      socket_errno = (r < 0) ? errno : 0;
-      if (r == 0 || socket_errno == ENOTSOCK)
-	ioctl (fd, FIONREAD, &r);
-# else
-      char data[64];
-      r = recv (fd, data, sizeof (data), MSG_PEEK);
-      socket_errno = (r < 0) ? errno : 0;
-# endif
-      if (r == 0)
-	happened |= POLLHUP;
-
-      /* If the event happened on an unconnected server socket,
-	 that's fine. */
-      else if (r > 0 || ( /* (r == -1) && */ socket_errno == ENOTCONN))
-	happened |= (POLLIN | POLLRDNORM) & sought;
-
-      /* Distinguish hung-up sockets from other errors.  */
-      else if (socket_errno == ESHUTDOWN || socket_errno == ECONNRESET
-	       || socket_errno == ECONNABORTED || socket_errno == ENETRESET)
-	happened |= POLLHUP;
-
-      else
-	happened |= POLLERR;
-    }
-
-  if (FD_ISSET (fd, wfds))
-    happened |= (POLLOUT | POLLWRNORM | POLLWRBAND) & sought;
-
-  if (FD_ISSET (fd, efds))
-    happened |= (POLLPRI | POLLRDBAND) & sought;
-
-  return happened;
-}
-#endif /* !MinGW */
-
-int
-poll (pfd, nfd, timeout)
-     struct pollfd *pfd;
-     nfds_t nfd;
-     int timeout;
-{
-#ifndef WIN32_NATIVE
-  fd_set rfds, wfds, efds;
-  struct timeval tv;
-  struct timeval *ptv;
-  int maxfd, rc;
-  nfds_t i;
-
-# ifdef _SC_OPEN_MAX
-  static int sc_open_max = -1;
-
-  if (nfd < 0
-      || (nfd > sc_open_max
-	  && (sc_open_max != -1
-	      || nfd > (sc_open_max = sysconf (_SC_OPEN_MAX)))))
-    {
-      errno = EINVAL;
-      return -1;
-    }
-# else /* !_SC_OPEN_MAX */
-#  ifdef OPEN_MAX
-  if (nfd < 0 || nfd > OPEN_MAX)
-    {
-      errno = EINVAL;
-      return -1;
-    }
-#  endif /* OPEN_MAX -- else, no check is needed */
-# endif /* !_SC_OPEN_MAX */
-
-  /* EFAULT is not necessary to implement, but let's do it in the
-     simplest case. */
-  if (!pfd)
-    {
-      errno = EFAULT;
-      return -1;
-    }
-
-  /* convert timeout number into a timeval structure */
-  if (timeout == 0)
-    {
-      ptv = &tv;
-      ptv->tv_sec = 0;
-      ptv->tv_usec = 0;
-    }
-  else if (timeout > 0)
-    {
-      ptv = &tv;
-      ptv->tv_sec = timeout / 1000;
-      ptv->tv_usec = (timeout % 1000) * 1000;
-    }
-  else if (timeout == INFTIM)
-    /* wait forever */
-    ptv = NULL;
-  else
-    {
-      errno = EINVAL;
-      return -1;
-    }
-
-  /* create fd sets and determine max fd */
-  maxfd = -1;
-  FD_ZERO (&rfds);
-  FD_ZERO (&wfds);
-  FD_ZERO (&efds);
-  for (i = 0; i < nfd; i++)
-    {
-      if (pfd[i].fd < 0)
-	continue;
-
-      if (pfd[i].events & (POLLIN | POLLRDNORM))
-	FD_SET (pfd[i].fd, &rfds);
-
-      /* see select(2): "the only exceptional condition detectable
-	 is out-of-band data received on a socket", hence we push
-	 POLLWRBAND events onto wfds instead of efds. */
-      if (pfd[i].events & (POLLOUT | POLLWRNORM | POLLWRBAND))
-	FD_SET (pfd[i].fd, &wfds);
-      if (pfd[i].events & (POLLPRI | POLLRDBAND))
-	FD_SET (pfd[i].fd, &efds);
-      if (pfd[i].fd >= maxfd
-	  && (pfd[i].events & (POLLIN | POLLOUT | POLLPRI
-			       | POLLRDNORM | POLLRDBAND
-			       | POLLWRNORM | POLLWRBAND)))
-	{
-	  maxfd = pfd[i].fd;
-	  if (maxfd > FD_SETSIZE)
-	    {
-	      errno = EOVERFLOW;
-	      return -1;
-	    }
-	}
-    }
-
-  /* examine fd sets */
-  rc = select (maxfd + 1, &rfds, &wfds, &efds, ptv);
-  if (rc < 0)
-    return rc;
-
-  /* establish results */
-  rc = 0;
-  for (i = 0; i < nfd; i++)
-    if (pfd[i].fd < 0)
-      pfd[i].revents = 0;
-    else
-      {
-	int happened = compute_revents (pfd[i].fd, pfd[i].events,
-					&rfds, &wfds, &efds);
-	if (happened)
-	  {
-	    pfd[i].revents = happened;
-	    rc++;
-	  }
-      }
-
-  return rc;
-#else
-  static struct timeval tv0;
-  static HANDLE hEvent;
-  WSANETWORKEVENTS ev;
-  HANDLE h, handle_array[FD_SETSIZE + 2];
-  DWORD ret, wait_timeout, nhandles;
-  fd_set rfds, wfds, xfds;
-  BOOL poll_again;
-  MSG msg;
-  int rc = 0;
-  nfds_t i;
-
-  if (nfd < 0 || timeout < -1)
-    {
-      errno = EINVAL;
-      return -1;
-    }
-
-  if (!hEvent)
-    hEvent = CreateEvent (NULL, FALSE, FALSE, NULL);
-
-  handle_array[0] = hEvent;
-  nhandles = 1;
-  FD_ZERO (&rfds);
-  FD_ZERO (&wfds);
-  FD_ZERO (&xfds);
-
-  /* Classify socket handles and create fd sets. */
-  for (i = 0; i < nfd; i++)
-    {
-      int sought = pfd[i].events;
-      pfd[i].revents = 0;
-      if (pfd[i].fd < 0)
-	continue;
-      if (!(sought & (POLLIN | POLLRDNORM | POLLOUT | POLLWRNORM | POLLWRBAND
-		      | POLLPRI | POLLRDBAND)))
-	continue;
-
-      h = (HANDLE) _get_osfhandle (pfd[i].fd);
-      assert (h != NULL);
-      if (IsSocketHandle (h))
-	{
-	  int requested = FD_CLOSE;
-
-	  /* see above; socket handles are mapped onto select.  */
-	  if (sought & (POLLIN | POLLRDNORM))
-	    {
-	      requested |= FD_READ | FD_ACCEPT;
-	      FD_SET ((SOCKET) h, &rfds);
-	    }
-	  if (sought & (POLLOUT | POLLWRNORM | POLLWRBAND))
-	    {
-	      requested |= FD_WRITE | FD_CONNECT;
-	      FD_SET ((SOCKET) h, &wfds);
-	    }
-	  if (sought & (POLLPRI | POLLRDBAND))
-	    {
-	      requested |= FD_OOB;
-	      FD_SET ((SOCKET) h, &xfds);
-	    }
-
-	  if (requested)
-	    WSAEventSelect ((SOCKET) h, hEvent, requested);
-	}
-      else
-	{
-	  /* Poll now.  If we get an event, do not poll again.  Also,
-	     screen buffer handles are waitable, and they'll block until
-	     a character is available.  win32_compute_revents eliminates
-	     bits for the "wrong" direction. */
-	  pfd[i].revents = win32_compute_revents (h, &sought);
-	  if (sought)
-	    handle_array[nhandles++] = h;
-	  if (pfd[i].revents)
-	    timeout = 0;
-	}
-    }
-
-  if (select (0, &rfds, &wfds, &xfds, &tv0) > 0)
-    {
-      /* Do MsgWaitForMultipleObjects anyway to dispatch messages, but
-	 no need to call select again.  */
-      poll_again = FALSE;
-      wait_timeout = 0;
-    }
-  else
-    {
-      poll_again = TRUE;
-      if (timeout == INFTIM)
-	wait_timeout = INFINITE;
-      else
-	wait_timeout = timeout;
-    }
-
-  for (;;)
-    {
-      ret = MsgWaitForMultipleObjects (nhandles, handle_array, FALSE,
-				       wait_timeout, QS_ALLINPUT);
-
-      if (ret == WAIT_OBJECT_0 + nhandles)
-	{
-	  /* new input of some other kind */
-	  BOOL bRet;
-	  while ((bRet = PeekMessage (&msg, NULL, 0, 0, PM_REMOVE)) != 0)
-	    {
-	      TranslateMessage (&msg);
-	      DispatchMessage (&msg);
-	    }
-	}
-      else
-	break;
-    }
-
-  if (poll_again)
-    select (0, &rfds, &wfds, &xfds, &tv0);
-
-  /* Place a sentinel at the end of the array.  */
-  handle_array[nhandles] = NULL;
-  nhandles = 1;
-  for (i = 0; i < nfd; i++)
-    {
-      int happened;
-
-      if (pfd[i].fd < 0)
-	continue;
-      if (!(pfd[i].events & (POLLIN | POLLRDNORM |
-			     POLLOUT | POLLWRNORM | POLLWRBAND)))
-	continue;
-
-      h = (HANDLE) _get_osfhandle (pfd[i].fd);
-      if (h != handle_array[nhandles])
-	{
-	  /* It's a socket.  */
-	  WSAEnumNetworkEvents ((SOCKET) h, NULL, &ev);
-	  WSAEventSelect ((SOCKET) h, 0, 0);
-
-	  /* If we're lucky, WSAEnumNetworkEvents already provided a way
-	     to distinguish FD_READ and FD_ACCEPT; this saves a recv later.  */
-	  if (FD_ISSET ((SOCKET) h, &rfds)
-	      && !(ev.lNetworkEvents & (FD_READ | FD_ACCEPT)))
-	    ev.lNetworkEvents |= FD_READ | FD_ACCEPT;
-	  if (FD_ISSET ((SOCKET) h, &wfds))
-	    ev.lNetworkEvents |= FD_WRITE | FD_CONNECT;
-	  if (FD_ISSET ((SOCKET) h, &xfds))
-	    ev.lNetworkEvents |= FD_OOB;
-
-	  happened = win32_compute_revents_socket ((SOCKET) h, pfd[i].events,
-						   ev.lNetworkEvents);
-	}
-      else
-	{
-	  /* Not a socket.  */
-	  int sought = pfd[i].events;
-	  happened = win32_compute_revents (h, &sought);
-	  nhandles++;
-	}
-
-       if ((pfd[i].revents |= happened) != 0)
-	rc++;
-    }
-
-  return rc;
-#endif
-}
diff --git a/compat/win32/sys/poll.h b/compat/win32/sys/poll.h
deleted file mode 100644
index b7aa59d..0000000
--- a/compat/win32/sys/poll.h
+++ /dev/null
@@ -1,53 +0,0 @@
-/* Header for poll(2) emulation
-   Contributed by Paolo Bonzini.
-
-   Copyright 2001, 2002, 2003, 2007, 2009, 2010 Free Software Foundation, Inc.
-
-   This file is part of gnulib.
-
-   This program is free software; you can redistribute it and/or modify
-   it under the terms of the GNU General Public License as published by
-   the Free Software Foundation; either version 2, or (at your option)
-   any later version.
-
-   This program is distributed in the hope that it will be useful,
-   but WITHOUT ANY WARRANTY; without even the implied warranty of
-   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-   GNU General Public License for more details.
-
-   You should have received a copy of the GNU General Public License along
-   with this program; if not, write to the Free Software Foundation,
-   Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.  */
-
-#ifndef _GL_POLL_H
-#define _GL_POLL_H
-
-/* fake a poll(2) environment */
-#define POLLIN      0x0001      /* any readable data available   */
-#define POLLPRI     0x0002      /* OOB/Urgent readable data      */
-#define POLLOUT     0x0004      /* file descriptor is writeable  */
-#define POLLERR     0x0008      /* some poll error occurred      */
-#define POLLHUP     0x0010      /* file descriptor was "hung up" */
-#define POLLNVAL    0x0020      /* requested events "invalid"    */
-#define POLLRDNORM  0x0040
-#define POLLRDBAND  0x0080
-#define POLLWRNORM  0x0100
-#define POLLWRBAND  0x0200
-
-struct pollfd
-{
-  int fd;                       /* which file descriptor to poll */
-  short events;                 /* events we are interested in   */
-  short revents;                /* events found on return        */
-};
-
-typedef unsigned long nfds_t;
-
-extern int poll (struct pollfd *pfd, nfds_t nfd, int timeout);
-
-/* Define INFTIM only if doing so conforms to POSIX.  */
-#if !defined (_POSIX_C_SOURCE) && !defined (_XOPEN_SOURCE)
-#define INFTIM (-1)
-#endif
-
-#endif /* _GL_POLL_H */
-- 
1.7.7.msysgit.1.1.g7b316

[PATCH v4 2/3] compat/win32/poll.c: upgrade from upstream

[Erik Faye-Lund]

poll.c is updated from revision adc3a5b in
git://git.savannah.gnu.org/gnulib.git

The changes are applied with --whitespace=fix to reduce noise.

poll.h is not upgraded, because the most recent version now
contains template-stuff that breaks compilation for us.

Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>
---
 compat/win32/poll.c |   19 +++++++++++++------
 1 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/compat/win32/poll.c b/compat/win32/poll.c
index 708a6c9..403eaa7 100644
--- a/compat/win32/poll.c
+++ b/compat/win32/poll.c
@@ -1,7 +1,7 @@
 /* Emulation for poll(2)
    Contributed by Paolo Bonzini.
 
-   Copyright 2001-2003, 2006-2010 Free Software Foundation, Inc.
+   Copyright 2001-2003, 2006-2011 Free Software Foundation, Inc.
 
    This file is part of gnulib.
 
@@ -27,7 +27,10 @@
 #include <malloc.h>
 
 #include <sys/types.h>
-#include "poll.h"
+
+/* Specification.  */
+#include <poll.h>
+
 #include <errno.h>
 #include <limits.h>
 #include <assert.h>
@@ -314,10 +317,7 @@ compute_revents (int fd, int sought, fd_set *rfds, fd_set *wfds, fd_set *efds)
 #endif /* !MinGW */
 
 int
-poll (pfd, nfd, timeout)
-     struct pollfd *pfd;
-     nfds_t nfd;
-     int timeout;
+poll (struct pollfd *pfd, nfds_t nfd, int timeout)
 {
 #ifndef WIN32_NATIVE
   fd_set rfds, wfds, efds;
@@ -454,6 +454,7 @@ poll (pfd, nfd, timeout)
   if (!hEvent)
     hEvent = CreateEvent (NULL, FALSE, FALSE, NULL);
 
+restart:
   handle_array[0] = hEvent;
   nhandles = 1;
   FD_ZERO (&rfds);
@@ -594,6 +595,12 @@ poll (pfd, nfd, timeout)
 	rc++;
     }
 
+  if (!rc && timeout == INFTIM)
+    {
+      SwitchToThread();
+      goto restart;
+    }
+
   return rc;
 #endif
 }
-- 
1.7.7.msysgit.1.1.g7b316

[PATCH v4 3/3] upload-archive: use start_command instead of fork

[Erik Faye-Lund]

The POSIX-function fork is not supported on Windows. Use our
start_command API instead.

As this is the last call-site that depends on the fork-stub in
compat/mingw.h, remove that as well.

Add an undocumented flag to git-archive that tells it that the
action originated from a remote, so features can be disabled.
Thanks to Jeff King for work on this part.

Remove the NOT_MINGW-prereq for t5000, as git-archive --remote
now works.

Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>
Helped-by: Jeff King <peff@peff.net>
---
 builtin/archive.c        |    6 +++-
 builtin/upload-archive.c |   68 ++++++++++++++-------------------------------
 compat/mingw.h           |    2 -
 t/t5000-tar-tree.sh      |   10 +++---
 4 files changed, 31 insertions(+), 55 deletions(-)

diff --git a/builtin/archive.c b/builtin/archive.c
index 931956d..e405566 100644
--- a/builtin/archive.c
+++ b/builtin/archive.c
@@ -87,6 +87,7 @@ int cmd_archive(int argc, const char **argv, const char *prefix)
 	const char *exec = "git-upload-archive";
 	const char *output = NULL;
 	const char *remote = NULL;
+	int is_remote = 0;
 	struct option local_opts[] = {
 		OPT_STRING('o', "output", &output, "file",
 			"write the archive to this file"),
@@ -94,6 +95,9 @@ int cmd_archive(int argc, const char **argv, const char *prefix)
 			"retrieve the archive from remote repository <repo>"),
 		OPT_STRING(0, "exec", &exec, "cmd",
 			"path to the remote git-upload-archive command"),
+		{ OPTION_BOOLEAN, 0, "remote-request", &is_remote, NULL,
+			"indicate we are serving a remote request",
+			PARSE_OPT_NOARG | PARSE_OPT_HIDDEN },
 		OPT_END()
 	};
 
@@ -108,5 +112,5 @@ int cmd_archive(int argc, const char **argv, const char *prefix)
 
 	setvbuf(stderr, NULL, _IOLBF, BUFSIZ);
 
-	return write_archive(argc, argv, prefix, 1, output, 0);
+	return write_archive(argc, argv, prefix, 1, output, is_remote);
 }
diff --git a/builtin/upload-archive.c b/builtin/upload-archive.c
index 2d0b383..c57e8bd 100644
--- a/builtin/upload-archive.c
+++ b/builtin/upload-archive.c
@@ -6,6 +6,7 @@
 #include "archive.h"
 #include "pkt-line.h"
 #include "sideband.h"
+#include "run-command.h"
 
 static const char upload_archive_usage[] =
 	"git upload-archive <repo>";
@@ -18,28 +19,17 @@ static const char lostchild[] =
 
 #define MAX_ARGS (64)
 
-static int run_upload_archive(int argc, const char **argv, const char *prefix)
+static void prepare_argv(const char **sent_argv, const char **argv)
 {
-	const char *sent_argv[MAX_ARGS];
 	const char *arg_cmd = "argument ";
 	char *p, buf[4096];
 	int sent_argc;
 	int len;
 
-	if (argc != 2)
-		usage(upload_archive_usage);
-
-	if (strlen(argv[1]) + 1 > sizeof(buf))
-		die("insanely long repository name");
-
-	strcpy(buf, argv[1]); /* enter-repo smudges its argument */
-
-	if (!enter_repo(buf, 0))
-		die("'%s' does not appear to be a git repository", buf);
-
 	/* put received options in sent_argv[] */
-	sent_argc = 1;
-	sent_argv[0] = "git-upload-archive";
+	sent_argc = 2;
+	sent_argv[0] = "archive";
+	sent_argv[1] = "--remote-request";
 	for (p = buf;;) {
 		/* This will die if not enough free space in buf */
 		len = packet_read_line(0, p, (buf + sizeof buf) - p);
@@ -62,9 +52,6 @@ static int run_upload_archive(int argc, const char **argv, const char *prefix)
 		*p++ = 0;
 	}
 	sent_argv[sent_argc] = NULL;
-
-	/* parse all options sent by the client */
-	return write_archive(sent_argc, sent_argv, prefix, 0, NULL, 1);
 }
 
 __attribute__((format (printf, 1, 2)))
@@ -96,38 +83,25 @@ static ssize_t process_input(int child_fd, int band)
 
 int cmd_upload_archive(int argc, const char **argv, const char *prefix)
 {
-	pid_t writer;
-	int fd1[2], fd2[2];
-	/*
-	 * Set up sideband subprocess.
-	 *
-	 * We (parent) monitor and read from child, sending its fd#1 and fd#2
-	 * multiplexed out to our fd#1.  If the child dies, we tell the other
-	 * end over channel #3.
-	 */
-	if (pipe(fd1) < 0 || pipe(fd2) < 0) {
-		int err = errno;
-		packet_write(1, "NACK pipe failed on the remote side\n");
-		die("upload-archive: %s", strerror(err));
-	}
-	writer = fork();
-	if (writer < 0) {
+	const char *sent_argv[MAX_ARGS];
+	struct child_process cld = { sent_argv };
+	cld.out = cld.err = -1;
+	cld.git_cmd = 1;
+
+	if (argc != 2)
+		usage(upload_archive_usage);
+
+	if (!enter_repo(argv[1], 0))
+		die("'%s' does not appear to be a git repository", argv[1]);
+
+	prepare_argv(sent_argv, argv);
+	if (start_command(&cld)) {
 		int err = errno;
 		packet_write(1, "NACK fork failed on the remote side\n");
 		die("upload-archive: %s", strerror(err));
 	}
-	if (!writer) {
-		/* child - connect fd#1 and fd#2 to the pipe */
-		dup2(fd1[1], 1);
-		dup2(fd2[1], 2);
-		close(fd1[1]); close(fd2[1]);
-		close(fd1[0]); close(fd2[0]); /* we do not read from pipe */
-
-		exit(run_upload_archive(argc, argv, prefix));
-	}
 
 	/* parent - read from child, multiplex and send out to fd#1 */
-	close(fd1[1]); close(fd2[1]); /* we do not write to pipe */
 	packet_write(1, "ACK\n");
 	packet_flush(1);
 
@@ -135,9 +109,9 @@ int cmd_upload_archive(int argc, const char **argv, const char *prefix)
 		struct pollfd pfd[2];
 		int status;
 
-		pfd[0].fd = fd1[0];
+		pfd[0].fd = cld.out;
 		pfd[0].events = POLLIN;
-		pfd[1].fd = fd2[0];
+		pfd[1].fd = cld.err;
 		pfd[1].events = POLLIN;
 		if (poll(pfd, 2, -1) < 0) {
 			if (errno != EINTR) {
@@ -156,7 +130,7 @@ int cmd_upload_archive(int argc, const char **argv, const char *prefix)
 			if (process_input(pfd[0].fd, 1))
 				continue;
 
-		if (waitpid(writer, &status, 0) < 0)
+		if (waitpid(cld.pid, &status, 0) < 0)
 			error_clnt("%s", lostchild);
 		else if (!WIFEXITED(status) || WEXITSTATUS(status) > 0)
 			error_clnt("%s", deadchild);
diff --git a/compat/mingw.h b/compat/mingw.h
index fecf0d0..dfb0e87 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -85,8 +85,6 @@ static inline int symlink(const char *oldpath, const char *newpath)
 { errno = ENOSYS; return -1; }
 static inline int fchmod(int fildes, mode_t mode)
 { errno = ENOSYS; return -1; }
-static inline pid_t fork(void)
-{ errno = ENOSYS; return -1; }
 static inline unsigned int alarm(unsigned int seconds)
 { return 0; }
 static inline int fsync(int fd)
diff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh
index d906898..889842e 100755
--- a/t/t5000-tar-tree.sh
+++ b/t/t5000-tar-tree.sh
@@ -96,7 +96,7 @@ test_expect_success 'git archive with --output' \
     'git archive --output=b4.tar HEAD &&
     test_cmp b.tar b4.tar'
 
-test_expect_success NOT_MINGW 'git archive --remote' \
+test_expect_success 'git archive --remote' \
     'git archive --remote=. HEAD >b5.tar &&
     test_cmp b.tar b5.tar'
 
@@ -266,7 +266,7 @@ test_expect_success 'archive --list mentions user filter' '
 	grep "^bar\$" output
 '
 
-test_expect_success NOT_MINGW 'archive --list shows only enabled remote filters' '
+test_expect_success 'archive --list shows only enabled remote filters' '
 	git archive --list --remote=. >output &&
 	! grep "^tar\.foo\$" output &&
 	grep "^bar\$" output
@@ -298,7 +298,7 @@ test_expect_success 'extension matching requires dot' '
 	test_cmp b.tar config-implicittar.foo
 '
 
-test_expect_success NOT_MINGW 'only enabled filters are available remotely' '
+test_expect_success 'only enabled filters are available remotely' '
 	test_must_fail git archive --remote=. --format=tar.foo HEAD \
 		>remote.tar.foo &&
 	git archive --remote=. --format=bar >remote.bar HEAD &&
@@ -341,12 +341,12 @@ test_expect_success GZIP,GUNZIP 'extract tgz file' '
 	test_cmp b.tar j.tar
 '
 
-test_expect_success GZIP,NOT_MINGW 'remote tar.gz is allowed by default' '
+test_expect_success GZIP 'remote tar.gz is allowed by default' '
 	git archive --remote=. --format=tar.gz HEAD >remote.tar.gz &&
 	test_cmp j.tgz remote.tar.gz
 '
 
-test_expect_success GZIP,NOT_MINGW 'remote tar.gz can be disabled' '
+test_expect_success GZIP 'remote tar.gz can be disabled' '
 	git config tar.tar.gz.remote false &&
 	test_must_fail git archive --remote=. --format=tar.gz HEAD \
 		>remote.tar.gz
-- 
1.7.7.msysgit.1.1.g7b316

Re: [PATCH v4 3/3] upload-archive: use start_command instead of fork

[Jeff King]

On Mon, Oct 24, 2011 at 06:02:11PM +0200, Erik Faye-Lund wrote:

> The POSIX-function fork is not supported on Windows. Use our
> start_command API instead.
> 
> As this is the last call-site that depends on the fork-stub in
> compat/mingw.h, remove that as well.
> 
> Add an undocumented flag to git-archive that tells it that the
> action originated from a remote, so features can be disabled.
> Thanks to Jeff King for work on this part.
> 
> Remove the NOT_MINGW-prereq for t5000, as git-archive --remote
> now works.
> 
> Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>
> Helped-by: Jeff King <peff@peff.net>
> ---

As with other versions of this series, I have no comment on the mingw
bits. The archive.c parts still look sane to me. :)

-Peff

Re: [PATCH v4 3/3] upload-archive: use start_command instead of fork

[Thomas Rast]

Erik Faye-Lund wrote:
> The POSIX-function fork is not supported on Windows. Use our
> start_command API instead.
> 
> As this is the last call-site that depends on the fork-stub in
> compat/mingw.h, remove that as well.
> 
> Add an undocumented flag to git-archive that tells it that the
> action originated from a remote, so features can be disabled.
> Thanks to Jeff King for work on this part.
> 
> Remove the NOT_MINGW-prereq for t5000, as git-archive --remote
> now works.

I see valgrind failures bisecting to this commit, like so:

==19125== Syscall param execve(argv[i]) points to unaddressable byte(s)
==19125==    at 0x5303CB7: execve (in /lib64/libc-2.11.3.so)
==19125==    by 0x53045A5: execvpe (in /lib64/libc-2.11.3.so)
==19125==    by 0x4B183C: execv_git_cmd (exec_cmd.c:137)
==19125==    by 0x4F305E: start_command (run-command.c:277)
==19125==    by 0x47F5C9: cmd_upload_archive (upload-archive.c:98)
==19125==    by 0x4051F4: run_builtin (git.c:308)
==19125==    by 0x40538F: handle_internal_command (git.c:466)
==19125==    by 0x405556: main (git.c:553)
==19125==  Address 0x7feffe7d0 is not stack'd, malloc'd or (recently) free'd

when running 'make valgrind' in current master.  Let me know if you
need more information.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

Re: [PATCH v4 3/3] upload-archive: use start_command instead of fork

[Jeff King]

On Tue, Nov 15, 2011 at 11:22:48AM +0100, Thomas Rast wrote:

> > Remove the NOT_MINGW-prereq for t5000, as git-archive --remote
> > now works.
> 
> I see valgrind failures bisecting to this commit, like so:
> 
> ==19125== Syscall param execve(argv[i]) points to unaddressable byte(s)
> ==19125==    at 0x5303CB7: execve (in /lib64/libc-2.11.3.so)
> ==19125==    by 0x53045A5: execvpe (in /lib64/libc-2.11.3.so)
> ==19125==    by 0x4B183C: execv_git_cmd (exec_cmd.c:137)
> ==19125==    by 0x4F305E: start_command (run-command.c:277)
> ==19125==    by 0x47F5C9: cmd_upload_archive (upload-archive.c:98)
> ==19125==    by 0x4051F4: run_builtin (git.c:308)
> ==19125==    by 0x40538F: handle_internal_command (git.c:466)
> ==19125==    by 0x405556: main (git.c:553)
> ==19125==  Address 0x7feffe7d0 is not stack'd, malloc'd or (recently) free'd
> 
> when running 'make valgrind' in current master.  Let me know if you
> need more information.

With which test, and on what OS? I couldn't replicate running
t5000 on Linux.

-Peff

Re: [PATCH v4 3/3] upload-archive: use start_command instead of fork

[Thomas Rast]

Jeff King wrote:
> On Tue, Nov 15, 2011 at 11:22:48AM +0100, Thomas Rast wrote:
> > 
> > I see valgrind failures bisecting to this commit, like so:
> > 
> > ==19125== Syscall param execve(argv[i]) points to unaddressable byte(s)
> > ==19125==    at 0x5303CB7: execve (in /lib64/libc-2.11.3.so)
> > ==19125==    by 0x53045A5: execvpe (in /lib64/libc-2.11.3.so)
> > ==19125==    by 0x4B183C: execv_git_cmd (exec_cmd.c:137)
> > ==19125==    by 0x4F305E: start_command (run-command.c:277)
> > ==19125==    by 0x47F5C9: cmd_upload_archive (upload-archive.c:98)
> > ==19125==    by 0x4051F4: run_builtin (git.c:308)
> > ==19125==    by 0x40538F: handle_internal_command (git.c:466)
> > ==19125==    by 0x405556: main (git.c:553)
> > ==19125==  Address 0x7feffe7d0 is not stack'd, malloc'd or (recently) free'd
> > 
> > when running 'make valgrind' in current master.  Let me know if you
> > need more information.
> 
> With which test, and on what OS? I couldn't replicate running
> t5000 on Linux.

11, and many others after it, on a pretty vanilla opensuse 11.4 VM I
use almost exclusively for the valgrind runs.  I used a pre-3.7 SVN
valgrind build, but 3.6.1 (shipped with opensuse) finds the same.

But after a closer look I think this patch just prodded it enough to
unearth long-existing undefined behaviour: prepare_argv() summarizes
to something like

static void prepare_argv(const char **sent_argv, const char **argv)
{
	char *p, buf[4096];

	for (p = buf;;) {
		len = packet_read_line(0, p, (buf + sizeof buf) - p);
		/* ... p always points into buf ... */
		sent_argv[sent_argc++] = p;
		p += len;
		*p++ = 0;
	}
	sent_argv[sent_argc] = NULL;
}

The code appears to have looked like this ever since the addition of
that file back in 39345a2 (Add git-upload-archive, 2006-09-07).  So
the elements of sent_argv have apparently always pointed into the
stack-allocated 'buf'.

(This correlates with the "Address 0x7feffe7d0 is not stack'd", even
though it's pretty clearly an address into the stack.)

A quick band-aid would be to heap-allocate it instead:

diff --git i/builtin/upload-archive.c w/builtin/upload-archive.c
index c57e8bd..6ab50d3 100644
--- i/builtin/upload-archive.c
+++ w/builtin/upload-archive.c
@@ -22,9 +22,10 @@ static const char lostchild[] =
 static void prepare_argv(const char **sent_argv, const char **argv)
 {
 	const char *arg_cmd = "argument ";
-	char *p, buf[4096];
+	char *p, *buf;
 	int sent_argc;
 	int len;
+	buf = xmalloc(4096);
 
 	/* put received options in sent_argv[] */
 	sent_argc = 2;
@@ -32,7 +33,7 @@ static void prepare_argv(const char **sent_argv, const char **argv)
 	sent_argv[1] = "--remote-request";
 	for (p = buf;;) {
 		/* This will die if not enough free space in buf */
-		len = packet_read_line(0, p, (buf + sizeof buf) - p);
+		len = packet_read_line(0, p, (buf + 4096) - p);
 		if (len == 0)
 			break;	/* got a flush */
 		if (sent_argc > MAX_ARGS - 2)

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

Re: [PATCH v4 3/3] upload-archive: use start_command instead of fork

[Jeff King]

On Tue, Nov 15, 2011 at 01:11:46PM +0100, Thomas Rast wrote:

> But after a closer look I think this patch just prodded it enough to
> unearth long-existing undefined behaviour: prepare_argv() summarizes
> to something like
> 
> static void prepare_argv(const char **sent_argv, const char **argv)
> {
> 	char *p, buf[4096];
> 
> 	for (p = buf;;) {
> 		len = packet_read_line(0, p, (buf + sizeof buf) - p);
> 		/* ... p always points into buf ... */
> 		sent_argv[sent_argc++] = p;
> 		p += len;
> 		*p++ = 0;
> 	}
> 	sent_argv[sent_argc] = NULL;
> }
> 
> The code appears to have looked like this ever since the addition of
> that file back in 39345a2 (Add git-upload-archive, 2006-09-07).  So
> the elements of sent_argv have apparently always pointed into the
> stack-allocated 'buf'.

Oh, yikes. That is definitely the problem, but it does come from
c09cd77e. The prepare_argv function used to be "run_upload_archive", and
it would prepare argv on the stack, call into write_archive with it, and
then return; nobody else cares about the value afterwards.

Erik's patch converts it into a function that writes the new argv into a
parameter and returns, and the now-invalid stack-allocated memory is
used by the calling function.

> A quick band-aid would be to heap-allocate it instead:

That works.  An even shorter band-aid is to mark it as "static".

I think the code would be more readable if it just used the new
argv_array.

Junio, this bug is in 1.7.8-rc*. Do you want the one-liner fix for the
release, or the nicer fix?

-Peff

Re: [PATCH v4 3/3] upload-archive: use start_command instead of fork

[Erik Faye-Lund]

On Tue, Nov 15, 2011 at 6:37 PM, Jeff King <peff@peff.net> wrote:
> On Tue, Nov 15, 2011 at 01:11:46PM +0100, Thomas Rast wrote:
>
>> But after a closer look I think this patch just prodded it enough to
>> unearth long-existing undefined behaviour: prepare_argv() summarizes
>> to something like
>>
>> static void prepare_argv(const char **sent_argv, const char **argv)
>> {
>>       char *p, buf[4096];
>>
>>       for (p = buf;;) {
>>               len = packet_read_line(0, p, (buf + sizeof buf) - p);
>>               /* ... p always points into buf ... */
>>               sent_argv[sent_argc++] = p;
>>               p += len;
>>               *p++ = 0;
>>       }
>>       sent_argv[sent_argc] = NULL;
>> }
>>
>> The code appears to have looked like this ever since the addition of
>> that file back in 39345a2 (Add git-upload-archive, 2006-09-07).  So
>> the elements of sent_argv have apparently always pointed into the
>> stack-allocated 'buf'.
>
> Oh, yikes. That is definitely the problem, but it does come from
> c09cd77e. The prepare_argv function used to be "run_upload_archive", and
> it would prepare argv on the stack, call into write_archive with it, and
> then return; nobody else cares about the value afterwards.
>
> Erik's patch converts it into a function that writes the new argv into a
> parameter and returns, and the now-invalid stack-allocated memory is
> used by the calling function.
>

Outch. Thanks for spotting.

>> A quick band-aid would be to heap-allocate it instead:
>
> That works.  An even shorter band-aid is to mark it as "static".

Hmm, I seem to remember spotting it myself at some point and fixing it
by marking it as static. I guess I must have forgot to push it...

> I think the code would be more readable if it just used the new
> argv_array.
>

Oooh, nice. The whole argv_array slipped past me, I like it!

Re: [PATCH v4 3/3] upload-archive: use start_command instead of fork

[Jeff King]

On Tue, Nov 15, 2011 at 06:44:38PM +0100, Erik Faye-Lund wrote:

> > I think the code would be more readable if it just used the new
> > argv_array.
> >
> Oooh, nice. The whole argv_array slipped past me, I like it!

I don't think it existed when you started on this series. :)

-Peff

Re: [PATCH v4 3/3] upload-archive: use start_command instead of fork

[Andreas Schwab]

Thomas Rast <trast@student.ethz.ch> writes:

> But after a closer look I think this patch just prodded it enough to
> unearth long-existing undefined behaviour: prepare_argv() summarizes
> to something like
>
> static void prepare_argv(const char **sent_argv, const char **argv)
> {
> 	char *p, buf[4096];
>
> 	for (p = buf;;) {
> 		len = packet_read_line(0, p, (buf + sizeof buf) - p);
> 		/* ... p always points into buf ... */
> 		sent_argv[sent_argc++] = p;
> 		p += len;
> 		*p++ = 0;
> 	}
> 	sent_argv[sent_argc] = NULL;
> }
>
> The code appears to have looked like this ever since the addition of
> that file back in 39345a2 (Add git-upload-archive, 2006-09-07).  So
> the elements of sent_argv have apparently always pointed into the
> stack-allocated 'buf'.
>
> (This correlates with the "Address 0x7feffe7d0 is not stack'd", even
> though it's pretty clearly an address into the stack.)
>
> A quick band-aid would be to heap-allocate it instead:

Or allocate it in the caller:

diff --git a/builtin/upload-archive.c b/builtin/upload-archive.c
index c57e8bd..f0f843e 100644
--- a/builtin/upload-archive.c
+++ b/builtin/upload-archive.c
@@ -18,11 +18,12 @@ static const char lostchild[] =
 "git upload-archive: archiver process was lost";
 
 #define MAX_ARGS (64)
+#define ARGV_BUF_SIZE 4096
 
-static void prepare_argv(const char **sent_argv, const char **argv)
+static void prepare_argv(const char **sent_argv, char *buf, const char **argv)
 {
 	const char *arg_cmd = "argument ";
-	char *p, buf[4096];
+	char *p;
 	int sent_argc;
 	int len;
 
@@ -32,7 +33,7 @@ static void prepare_argv(const char **sent_argv, const char **argv)
 	sent_argv[1] = "--remote-request";
 	for (p = buf;;) {
 		/* This will die if not enough free space in buf */
-		len = packet_read_line(0, p, (buf + sizeof buf) - p);
+		len = packet_read_line(0, p, (buf + ARGV_BUF_SIZE) - p);
 		if (len == 0)
 			break;	/* got a flush */
 		if (sent_argc > MAX_ARGS - 2)
@@ -85,6 +86,7 @@ int cmd_upload_archive(int argc, const char **argv, const char *prefix)
 {
 	const char *sent_argv[MAX_ARGS];
 	struct child_process cld = { sent_argv };
+	char argv_buf[ARGV_BUF_SIZE];
 	cld.out = cld.err = -1;
 	cld.git_cmd = 1;
 
@@ -94,7 +96,7 @@ int cmd_upload_archive(int argc, const char **argv, const char *prefix)
 	if (!enter_repo(argv[1], 0))
 		die("'%s' does not appear to be a git repository", argv[1]);
 
-	prepare_argv(sent_argv, argv);
+	prepare_argv(sent_argv, argv_buf, argv);
 	if (start_command(&cld)) {
 		int err = errno;
 		packet_write(1, "NACK fork failed on the remote side\n");
-- 
1.7.7.3


Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [PATCH v4 3/3] upload-archive: use start_command instead of fork

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> Junio, this bug is in 1.7.8-rc*. Do you want the one-liner fix for the
> release, or the nicer fix?

Let's just do "static" for now, if we know the array is large enough.

Re: [PATCH v4 3/3] upload-archive: use start_command instead of fork

[Jeff King]

On Tue, Nov 15, 2011 at 10:59:47AM -0800, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > Junio, this bug is in 1.7.8-rc*. Do you want the one-liner fix for the
> > release, or the nicer fix?
> 
> Let's just do "static" for now, if we know the array is large enough.

OK, here it is.

I think it's correct, but I couldn't reproduce the valgrind failure
here. Thomas, can you double check that this also solves your problem?

-Peff

-- >8 --
Subject: [PATCH] upload-archive: don't return pointers to stack buffer

The prepare_argv function uses an internal stack-allocated
buffer to create the argv array that will be used to run
git-archive. Prior to c09cd77e, this was OK, as the
function passed the argv array to write_archive itself, and
the stack buffer was valid during its use.

Since c09cd77e, however, the function returns an argv array
with pointers pointing into the stack buffer. The calling
function then passes the result to start_command, which
tries to execve using pointers to a now-invalid buffer.

Fix it by making the buffer static, which is a quick and
simple fix, and works because we only run this function once
per process.

Credit for finding the bug and most of the analysis goes to
Thomas Rast.

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin/upload-archive.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/builtin/upload-archive.c b/builtin/upload-archive.c
index c57e8bd..f47c0f0 100644
--- a/builtin/upload-archive.c
+++ b/builtin/upload-archive.c
@@ -22,7 +22,8 @@
 static void prepare_argv(const char **sent_argv, const char **argv)
 {
 	const char *arg_cmd = "argument ";
-	char *p, buf[4096];
+	char *p;
+	static char buf[4096];
 	int sent_argc;
 	int len;
 
-- 
1.7.7.3.8.g38efa

[PATCH 1/2] upload-archive: drop extra argument to prepare_argv

[Jeff King]

We pass a "sent_argv" which is an out-parameter to hold the
argv that we were sent over the wire. But we also pass in
the "argv" we got on the command line, which is not used at
all. Drop this useless and confusing parameter.

The parameter was obsoleted by c09cd77e, which moved the
enter_repo function (which looked at argv[1]) out of
prepare_argv and into cmd_upload_archive.

Signed-off-by: Jeff King <peff@peff.net>
---
On Tue, Nov 15, 2011 at 02:18:32PM -0500, Jeff King wrote:

> > Let's just do "static" for now, if we know the array is large enough.
> 
> OK, here it is.

And here's the other more invasive cleanup on top (patch 2 is the meaty
one).

 builtin/upload-archive.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/builtin/upload-archive.c b/builtin/upload-archive.c
index f47c0f0..80575b9 100644
--- a/builtin/upload-archive.c
+++ b/builtin/upload-archive.c
@@ -19,7 +19,7 @@
 
 #define MAX_ARGS (64)
 
-static void prepare_argv(const char **sent_argv, const char **argv)
+static void prepare_argv(const char **sent_argv)
 {
 	const char *arg_cmd = "argument ";
 	char *p;
@@ -95,7 +95,7 @@ int cmd_upload_archive(int argc, const char **argv, const char *prefix)
 	if (!enter_repo(argv[1], 0))
 		die("'%s' does not appear to be a git repository", argv[1]);
 
-	prepare_argv(sent_argv, argv);
+	prepare_argv(sent_argv);
 	if (start_command(&cld)) {
 		int err = errno;
 		packet_write(1, "NACK fork failed on the remote side\n");
-- 
1.7.7.3.8.g38efa

[PATCH] upload-archive: use argv_array for sent parameters

[Jeff King]

The existing prepare_argv uses a fixed-size static buffer to
hold all of the arguments, and then puts pointers into the
buffer into a fixed-size array. Using argv_array gets rid of
all of the manual bookkeeping and makes the code more
readable.

It also lifts the static limits on the size of the array.
This is convenient, but is perhaps a security regression, as
a malicious client can now ask us to create arbitrary-length
argv arrays in memory.

Signed-off-by: Jeff King <peff@peff.net>
---
I think the code is way more readable and obvious.  Do we care about the
potential DoS?  If so, we can still cap the number of arguments we'll
accept. In practice, we never send more than a few.

 builtin/upload-archive.c |   45 +++++++++++++++++----------------------------
 1 files changed, 17 insertions(+), 28 deletions(-)

diff --git a/builtin/upload-archive.c b/builtin/upload-archive.c
index 80575b9..3830310 100644
--- a/builtin/upload-archive.c
+++ b/builtin/upload-archive.c
@@ -7,6 +7,7 @@
 #include "pkt-line.h"
 #include "sideband.h"
 #include "run-command.h"
+#include "argv-array.h"
 
 static const char upload_archive_usage[] =
 	"git upload-archive <repo>";
@@ -17,42 +18,28 @@
 static const char lostchild[] =
 "git upload-archive: archiver process was lost";
 
-#define MAX_ARGS (64)
-
-static void prepare_argv(const char **sent_argv)
+static void prepare_argv(struct argv_array *out)
 {
 	const char *arg_cmd = "argument ";
-	char *p;
-	static char buf[4096];
-	int sent_argc;
-	int len;
 
-	/* put received options in sent_argv[] */
-	sent_argc = 2;
-	sent_argv[0] = "archive";
-	sent_argv[1] = "--remote-request";
-	for (p = buf;;) {
+	argv_array_push(out, "archive");
+	argv_array_push(out, "--remote-request");
+	while (1) {
+		char buf[4096];
+		int len;
 		/* This will die if not enough free space in buf */
-		len = packet_read_line(0, p, (buf + sizeof buf) - p);
+		len = packet_read_line(0, buf, sizeof(buf));
 		if (len == 0)
 			break;	/* got a flush */
-		if (sent_argc > MAX_ARGS - 2)
-			die("Too many options (>%d)", MAX_ARGS - 2);
 
-		if (p[len-1] == '\n') {
-			p[--len] = 0;
-		}
+		if (buf[len-1] == '\n')
+			buf[--len] = 0;
 		if (len < strlen(arg_cmd) ||
-		    strncmp(arg_cmd, p, strlen(arg_cmd)))
+		    strncmp(arg_cmd, buf, strlen(arg_cmd)))
 			die("'argument' token or flush expected");
 
-		len -= strlen(arg_cmd);
-		memmove(p, p + strlen(arg_cmd), len);
-		sent_argv[sent_argc++] = p;
-		p += len;
-		*p++ = 0;
+		argv_array_push(out, buf + strlen(arg_cmd));
 	}
-	sent_argv[sent_argc] = NULL;
 }
 
 __attribute__((format (printf, 1, 2)))
@@ -84,8 +71,8 @@ static ssize_t process_input(int child_fd, int band)
 
 int cmd_upload_archive(int argc, const char **argv, const char *prefix)
 {
-	const char *sent_argv[MAX_ARGS];
-	struct child_process cld = { sent_argv };
+	struct argv_array sent_argv = ARGV_ARRAY_INIT;
+	struct child_process cld = {0};
 	cld.out = cld.err = -1;
 	cld.git_cmd = 1;
 
@@ -95,7 +82,8 @@ int cmd_upload_archive(int argc, const char **argv, const char *prefix)
 	if (!enter_repo(argv[1], 0))
 		die("'%s' does not appear to be a git repository", argv[1]);
 
-	prepare_argv(sent_argv);
+	prepare_argv(&sent_argv);
+	cld.argv = sent_argv.argv;
 	if (start_command(&cld)) {
 		int err = errno;
 		packet_write(1, "NACK fork failed on the remote side\n");
@@ -138,5 +126,6 @@ int cmd_upload_archive(int argc, const char **argv, const char *prefix)
 		packet_flush(1);
 		break;
 	}
+	argv_array_clear(&sent_argv);
 	return 0;
 }
-- 
1.7.7.3.8.g38efa

Re: [PATCH] upload-archive: use argv_array for sent parameters

[Jeff King]

On Tue, Nov 15, 2011 at 02:49:58PM -0500, Jeff King wrote:

> The existing prepare_argv uses a fixed-size static buffer to
> hold all of the arguments, and then puts pointers into the
> buffer into a fixed-size array. Using argv_array gets rid of
> all of the manual bookkeeping and makes the code more
> readable.
> 
> It also lifts the static limits on the size of the array.
> This is convenient, but is perhaps a security regression, as
> a malicious client can now ask us to create arbitrary-length
> argv arrays in memory.
> 
> Signed-off-by: Jeff King <peff@peff.net>

Sorry, I failed to mark the subject properly. If it was not obvious,
this is patch 2/2 of the cleanup.

-Peff