Mailing-List: contact cygwin-developers-help AT sourceware DOT cygnus DOT com; run by ezmlm
List-Unsubscribe: <mailto:cygwin-developers-unsubscribe-archive-cygwin-developers=delorie DOT com AT sourceware DOT cygnus DOT com>
List-Subscribe: <mailto:cygwin-developers-subscribe AT sourceware DOT cygnus DOT com>
List-Archive: <http://sourceware.cygnus.com/ml/cygwin-developers/>
List-Post: <mailto:cygwin-developers AT sourceware DOT cygnus DOT com>
List-Help: <mailto:cygwin-developers-help AT sourceware DOT cygnus DOT com>, <http://sourceware.cygnus.com/ml/#faqs>
Sender: cygwin-developers-owner AT sourceware DOT cygnus DOT com
Delivered-To: mailing list cygwin-developers AT sourceware DOT cygnus DOT com
Message-Id: <199910070036.RAA20891@herra.corp.netapp.com>
To: cygwin-developers AT sourceware DOT cygnus DOT com
Subject: patch submission: fixes in find_unused_handle() and cygwin_select()
Date: Wed, 06 Oct 1999 17:36:58 -0700
From: Lincoln Myers <lincoln AT netapp DOT com>


Here is a patch for two problems (and some printfs which helped debug
the second problem) that I found in trying to get a perl script to
work which makes rather heavy use of IPC::Open3 to start many children
and talk to their stdin/stdout/stderr.  The patch is to B20.1, though
it looked like this stuff is no different in the 19991003 snapshot.

Changes:

	- hinfo::find_unused_handle() now accepts a start one greater
	  than any existing descriptor, something which make_pipe()
	  sometimes needs for the write fd, if the read fd was the
	  last allocated.

	- hinfo::select_{read,write,execute}() now select_printf()s what
	  descriptor is bad when there's a bad file descriptor.  I
	  found this quite helpful in debugging the problem below, and
	  wished it were already there.

	- cygwin_select() now handles fd_sets whose size is based on
	  the fd count passed in, rather than sizeof(fd_set).  I need
	  this because the default FD_SETSIZE of 64 is too low for me,
	  and perl and presumably other callers ignore the defined
	  fd_set and roll their own possibly smaller or larger bit
	  vectors anyway.

	  In particular:

		- the fd_set allocated in place of a NULL fd_set
		  argument to cygwin_select() is now sized properly if
		  n>FD_SETSIZE.

		- select_stuff::wait() no longer uses a separate copy
		  of the fd_sets given as arguments.  If I understood
		  the comment above cygwin_select(), it used to do
		  this because the fd_sets given to cygwin_select()
		  might not be a complete sizeof(fd_set) in size, but
		  then select_stuff::wait() used to do a structure
		  copy of r,w,s on top of readfds,writefds,exceptfds,
		  so cygwin_select() was not really handling smaller
		  fd_sets.  Consequently, I'm pretty sure my changes
		  handle caller's arguments at least as gingerly, and
		  it seems to be perl-ly correct, though I'm only
		  hoping that implies it to be posix-ly correct...

	  Y'all who know C++ better than I do might have a much better
	  idea about how dummy_{read,write,except}fds should be
	  allocated.

----------------------------------------------------------------------

diff -u -r /home/lincoln/cygwin/B20.1/src/winsup/hinfo.cc winsup/hinfo.cc
--- /home/lincoln/cygwin/B20.1/src/winsup/hinfo.cc	Mon Oct  4 21:48:34 1999
+++ winsup/hinfo.cc	Tue Oct  5 21:09:26 1999
@@ -113,7 +113,7 @@
 int
 hinfo::find_unused_handle (int start)
 {
-  if ((size_t)start >= size)
+  if ((size_t)start > size)
     return -1;
 
   do
@@ -322,6 +322,7 @@
   if (dtable.not_open (fd))
     {
       set_errno (EBADF);
+      select_printf("select_read: fd %d not open",fd);
       return NULL;
     }
   fhandler_base *fh = dtable[fd];
@@ -338,6 +339,7 @@
   if (dtable.not_open (fd))
     {
       set_errno (EBADF);
+      select_printf("select_write: fd %d not open",fd);
       return NULL;
     }
   fhandler_base *fh = dtable[fd];
@@ -354,6 +356,7 @@
   if (dtable.not_open (fd))
     {
       set_errno (EBADF);
+      select_printf("select_except: fd %d not open",fd);
       return NULL;
     }
   fhandler_base *fh = dtable[fd];
diff -u -r /home/lincoln/cygwin/B20.1/src/winsup/select.cc winsup/select.cc
--- /home/lincoln/cygwin/B20.1/src/winsup/select.cc	Mon Oct  4 21:48:35 1999
+++ winsup/select.cc	Tue Oct  5 21:06:31 1999
@@ -68,13 +68,19 @@
 
 #define unix_fd_set fd_set
 
+/* how many fd_masks needed to hold this many fds? */
+#define UNIX_NFDMASKS(n) (unix_howmany((n),UNIX_NFDBITS))
+
+/* how many bytes needed to hold the fd_masks needed to hold this many fds? */
+#define UNIX_MASKSIZE(n) (sizeof(fd_mask)*(UNIX_NFDMASKS(n)))
+
 #define UNIX_FD_SET(n, p) \
 	 ((p)->fds_bits[(n)/UNIX_NFDBITS] |= (1L << ((n) % UNIX_NFDBITS)))
 #define UNIX_FD_CLR(n, p) \
 	 ((p)->fds_bits[(n)/UNIX_NFDBITS] &= ~(1L << ((n) % UNIX_NFDBITS)))
 #define UNIX_FD_ISSET(n, p) \
 	 ((p)->fds_bits[(n)/UNIX_NFDBITS] & (1L << ((n) % UNIX_NFDBITS)))
-#define UNIX_FD_ZERO(p)      bzero ((caddr_t)(p), sizeof (*(p)))
+#define UNIX_FD_ZERO(p,n)    bzero ((caddr_t)(p), (UNIX_MASKSIZE(n)));
 
 #define MAKEready(what) \
 int \
@@ -88,19 +94,32 @@
   return me.read_ready; \
 }
 
+static fd_set *
+alloc_empty_fd_set(int n)
+{
+  fd_set *f = (fd_set *)malloc(UNIX_MASKSIZE(n));
+  if (f) {
+    UNIX_FD_ZERO(f,n);
+  }
+  return f;
+}
+
 /*
- * The main select code. The fd_set pointers here are not taken as
- * given as we are not sure how large the fd_set arrays are in the client code.
- * We only look as far as the largest value of the n parameter.
+ * The main select code. The fd_set pointers are treated as just large
+ * enough (rounded up to the nearest fd_mask i.e. long integer) to
+ * hold the 'n' given file descriptors.
  */
 extern "C"
 int
 cygwin_select (int n, fd_set *readfds, fd_set *writefds, fd_set *exceptfds,
 		     struct timeval *to)
 {
   sig_protect (here, 1);
+  DWORD ms;
   select_stuff sel;
-  fd_set dummy_readfds, dummy_writefds, dummy_exceptfds;
+  fd_set *dummy_readfds=NULL, *dummy_writefds=NULL, *dummy_exceptfds=NULL;
+  int result=-1;
 
   ResetEvent (signal_arrived);
   select_printf ("%d, %p, %p, %p, %p", n, readfds, writefds, exceptfds, to);
@@ -108,28 +127,38 @@
   memset (&sel, 0, sizeof (sel));
   if (!readfds)
     {
-      UNIX_FD_ZERO (&dummy_readfds);
-      readfds = &dummy_readfds;
+      readfds = dummy_readfds = alloc_empty_fd_set(n);
+      if (!readfds) {
+	set_errno (ENOMEM);
+	goto out;
+      }
     }
   if (!writefds)
     {
-      UNIX_FD_ZERO (&dummy_writefds);
-      writefds = &dummy_writefds;
+      writefds = dummy_writefds = alloc_empty_fd_set(n);
+      if (!writefds) {
+	set_errno (ENOMEM);
+	goto out;
+      }
     }
   if (!exceptfds)
     {
-      UNIX_FD_ZERO (&dummy_exceptfds);
-      exceptfds = &dummy_exceptfds;
+      exceptfds = dummy_exceptfds = alloc_empty_fd_set(n);
+      if (!exceptfds) {
+	set_errno (ENOMEM);
+	goto out;
+      }
     }
 
   for (int i = 0; i < n; i++)
     if (!sel.test_and_set (i, readfds, writefds, exceptfds))
       {
 	select_printf ("aborting due to test_and_set error");
-	return -1;	/* Invalid fd, maybe? */
+	result=-1;	/* Invalid fd, maybe? */
+	goto out;
       }
 
-  DWORD ms = to ? (to->tv_sec * 1000) + (to->tv_usec / 1000) : INFINITE;
+  ms = to ? (to->tv_sec * 1000) + (to->tv_usec / 1000) : INFINITE;
   if (ms == 0 && to->tv_usec)
     ms = 1;			/* At least 1 ms granularity */
 
@@ -143,18 +172,30 @@
   if (sel.total == 0)
     {
       Sleep (ms);
-      return 0;
+      result=0;
+      goto out;
     }
 
+  UNIX_FD_ZERO (readfds,n);
+  UNIX_FD_ZERO (writefds,n);
+  UNIX_FD_ZERO (exceptfds,n);
   if (sel.always_ready || ms == 0)
     {
-      UNIX_FD_ZERO (readfds);
-      UNIX_FD_ZERO (writefds);
-      UNIX_FD_ZERO (exceptfds);
-      return sel.poll (readfds, writefds, exceptfds);
+      result=sel.poll (readfds, writefds, exceptfds);
+    }
+  else
+    {
+      result=sel.wait (readfds, writefds, exceptfds, ms);
     }
 
-  return sel.wait (readfds, writefds, exceptfds, ms);
+ out:
+  if (dummy_readfds!=NULL)
+    free(dummy_readfds);
+  if (dummy_writefds!=NULL)
+    free(dummy_writefds);
+  if (dummy_exceptfds!=NULL)
+    free(dummy_exceptfds);
+  return result;
 }
 
 select_stuff::~select_stuff ()
@@ -242,10 +283,6 @@
     }
 
   DWORD start_time = GetTickCount ();
-  fd_set r, w, e;
-  UNIX_FD_ZERO (&r);
-  UNIX_FD_ZERO (&w);
-  UNIX_FD_ZERO (&e);
   debug_printf ("n %d, ms %u", n, ms);
   for (;;)
     {
@@ -274,7 +311,7 @@
       int gotone = FALSE;
       while ((s = s->next))
 	if ((((wait_ret >= n && s->windows_handle) || s->h == w4[wait_ret])) &&
-	    s->verify (s, &r, &w, &e))
+	    s->verify (s, readfds, writefds, exceptfds))
 	  gotone = TRUE;
 
       select_printf ("gotone %d", gotone);
@@ -300,10 +337,6 @@
     }
 
 out:
-  *readfds = r;
-  *writefds = w;
-  *exceptfds = e;
-
   return poll (readfds, writefds, exceptfds);
 }
 
----------------------------------------------------------------------

Lincoln