2 files changed, 117 insertions, 94 deletions

diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h
index d3afd753a..64a052ce1 100644
--- a/winsup/cygwin/fhandler.h
+++ b/winsup/cygwin/fhandler.h
@@ -1680,8 +1680,8 @@ class fhandler_cygdrive: public fhandler_disk_file
 class fhandler_serial: public fhandler_base
 {
  private:
-  size_t vmin_;				/* from termios */
-  unsigned int vtime_;			/* from termios */
+  cc_t vmin_;				/* from termios */
+  cc_t vtime_;				/* from termios */
   pid_t pgrp_;
   int rts;				/* for Windows 9x purposes only */
   int dtr;				/* for Windows 9x purposes only */
@@ -1689,7 +1689,6 @@ class fhandler_serial: public fhandler_base
  public:
   int overlapped_armed;
   OVERLAPPED io_status;
-  DWORD ev;
 
   /* Constructor */
   fhandler_serial ();
diff --git a/winsup/cygwin/fhandler_serial.cc b/winsup/cygwin/fhandler_serial.cc
index f729765e0..7492470f5 100644
--- a/winsup/cygwin/fhandler_serial.cc
+++ b/winsup/cygwin/fhandler_serial.cc
@@ -41,12 +41,21 @@ fhandler_serial::overlapped_setup ()
 void __reg3
 fhandler_serial::raw_read (void *ptr, size_t& ulen)
 {
-  int tot;
-  DWORD n;
+  DWORD io_err, event;
+  COMSTAT st;
+  DWORD bytes_to_read, read_bytes, undefined;
+  ssize_t tot = 0;
+
+  if (ulen > SSIZE_MAX)
+    ulen = SSIZE_MAX;
+  if (ulen == 0)
+    return;
 
-  size_t minchars = vmin_ ? MIN (vmin_, ulen) : ulen;
+  /* If VMIN > 0 in blocking mode, we have to read at least VMIN chars,
+     otherwise we're in polling mode and there's no minimum chars. */
+  ssize_t minchars = (!is_nonblocking () && vmin_) ? MIN (vmin_, ulen) : 0;
 
-  debug_printf ("ulen %ld, vmin_ %ld, vtime_ %u, hEvent %p",
+  debug_printf ("ulen %ld, vmin_ %u, vtime_ %u, hEvent %p",
 		ulen, vmin_, vtime_, io_status.hEvent);
   if (!overlapped_armed)
     {
@@ -54,122 +63,137 @@ fhandler_serial::raw_read (void *ptr, size_t& ulen)
       ResetEvent (io_status.hEvent);
     }
 
-  for (n = 0, tot = 0; ulen; ulen -= n, ptr = (char *) ptr + n)
+  do
     {
-      COMSTAT st;
-      DWORD inq = vmin_ ? minchars : vtime_ ? ulen : 1;
-
-      n = 0;
-
-      if (vtime_) // non-interruptible -- have to use kernel timeouts
-	overlapped_armed = -1;
-
-      if (!ClearCommError (get_handle (), &ev, &st))
+      /* First check if chars are already in the inbound queue. */
+      if (!ClearCommError (get_handle (), &io_err, &st))
 	goto err;
-      else if (ev)
-	termios_printf ("error detected %x", ev);
-      else if (is_nonblocking ())
+      /* FIXME: In case of I/O error, do we really want to bail out or is it
+		better just trying to pull through? */
+      if (io_err)
 	{
-	  if (!st.cbInQue)
-	    {
-	      tot = -1;
-	      set_errno (EAGAIN);
-	      goto out;
-	    }
-	  inq = st.cbInQue;
+	  termios_printf ("error detected %x", io_err);
+	  SetLastError (ERROR_IO_DEVICE);
+	  goto err;
 	}
-      else if (st.cbInQue && !vtime_)
-	inq = st.cbInQue;
-      else if (!is_nonblocking () && !overlapped_armed)
+      /* ReadFile only handles up to DWORD bytes. */
+      bytes_to_read = MIN (ulen, UINT32_MAX);
+      if (is_nonblocking ())
 	{
-	  if ((size_t) tot >= minchars)
+	  /* In O_NONBLOCK mode we just care for the number of chars already
+	     in the inbound queue. */
+	  if (!st.cbInQue)
 	    break;
-	  else if (WaitCommEvent (get_handle (), &ev, &io_status))
+	  bytes_to_read = MIN (st.cbInQue, bytes_to_read);
+	}
+      else
+	{
+	  /* If the number of chars in the inbound queue is sufficent
+	     (minchars defines the minimum), set bytes_to_read accordingly
+	     and don't wait. */
+	  if (st.cbInQue && st.cbInQue >= minchars)
+	    bytes_to_read = MIN (st.cbInQue, bytes_to_read);
+	  /* if vtime_ is set, use kernel timeouts, otherwise wait here. */
+	  else if (vtime_ == 0 && !overlapped_armed)
 	    {
-	      debug_printf ("WaitCommEvent succeeded: ev %x", ev);
-	      if (!ev)
-		continue;
+	      if (WaitCommEvent (get_handle (), &event, &io_status))
+		{
+		  debug_printf ("WaitCommEvent succeeded: event %x", event);
+		  if (!event)
+		    continue;
+		}
+	      else if (GetLastError () != ERROR_IO_PENDING)
+		goto err;
+	      overlapped_armed = 1;
 	    }
-	  else if (GetLastError () != ERROR_IO_PENDING)
-	    goto err;
-	  else
+	}
+      /* overlapped_armed may be set by select, so we have to make sure
+	 to disarm even in O_NONBLOCK mode. */
+      if (overlapped_armed)
+	{
+	  switch (cygwait (io_status.hEvent, is_nonblocking () ? 0 : INFINITE))
 	    {
-	      overlapped_armed = 1;
-	      switch (cygwait (io_status.hEvent))
+	    case WAIT_OBJECT_0:
+	      if (!GetOverlappedResult (get_handle (), &io_status,
+					&undefined, TRUE))
+		goto err;
+	      debug_printf ("overlapped event %x", event);
+	      break;
+	    case WAIT_SIGNALED:
+	      CancelIo (get_handle ());
+	      overlapped_armed = 0;
+	      if (!GetOverlappedResult (get_handle (), &io_status,
+					&undefined, TRUE))
+		goto err;
+	      /* Only if no bytes read, return with EINTR. */
+	      if (!tot)
 		{
-		case WAIT_OBJECT_0:
-		  if (!GetOverlappedResult (get_handle (), &io_status, &n,
-					    FALSE))
-		    goto err;
-		  debug_printf ("n %u, ev %x", n, ev);
-		  break;
-		case WAIT_SIGNALED:
 		  tot = -1;
-		  PurgeComm (get_handle (), PURGE_RXABORT);
-		  overlapped_armed = 0;
 		  set_sig_errno (EINTR);
-		  goto out;
-		case WAIT_CANCELED:
-		  PurgeComm (get_handle (), PURGE_RXABORT);
-		  overlapped_armed = 0;
-		  pthread::static_cancel_self ();
-		  /*NOTREACHED*/
-		default:
-		  goto err;
 		}
+	      goto out;
+	    case WAIT_CANCELED:
+	      CancelIo (get_handle ());
+	      overlapped_armed = 0;
+	      GetOverlappedResult (get_handle (), &io_status, &undefined,
+				   TRUE);
+	      pthread::static_cancel_self ();
+	      /*NOTREACHED*/
+	    default:
+	      goto err;
 	    }
 	}
 
       overlapped_armed = 0;
       ResetEvent (io_status.hEvent);
-      if (inq > ulen)
-	inq = ulen;
-      debug_printf ("inq %u", inq);
-      if (ReadFile (get_handle (), ptr, inq, &n, &io_status))
-	/* Got something */;
-      else if (GetLastError () != ERROR_IO_PENDING)
-	goto err;
-      else if (is_nonblocking ())
+      if (!ReadFile (get_handle (), ptr, bytes_to_read, &read_bytes,
+		     &io_status))
 	{
-	  /* Use CancelIo rather than PurgeComm (PURGE_RXABORT) since
-	     PurgeComm apparently discards in-flight bytes while CancelIo
-	     only stops the overlapped IO routine. */
-	  CancelIo (get_handle ());
-	  if (GetOverlappedResult (get_handle (), &io_status, &n, TRUE))
-	    tot = n;
-	  else if (GetLastError () != ERROR_OPERATION_ABORTED)
+	  if (GetLastError () != ERROR_IO_PENDING)
+	    goto err;
+	  if (is_nonblocking ())
+	    CancelIo (get_handle ());
+	  if (!GetOverlappedResult (get_handle (), &io_status, &read_bytes,
+				    TRUE))
 	    goto err;
-	  if (tot == 0)
-	    {
-	      tot = -1;
-	      set_errno (EAGAIN);
-	    }
-	  goto out;
 	}
-      else if (!GetOverlappedResult (get_handle (), &io_status, &n, TRUE))
-	goto err;
-
-      tot += n;
-      debug_printf ("vtime_ %u, vmin_ %lu, n %u, tot %d", vtime_, vmin_, n, tot);
-      if (vtime_ || !vmin_ || !n)
-	break;
+      tot += read_bytes;
+      ptr = (void *) ((caddr_t) ptr + read_bytes);
+      ulen -= read_bytes;
+      minchars -= read_bytes;
+      debug_printf ("vtime_ %u, vmin_ %u, read_bytes %u, tot %D",
+		    vtime_, vmin_, read_bytes, tot);
       continue;
 
     err:
       debug_printf ("err %E");
       if (GetLastError () != ERROR_OPERATION_ABORTED)
 	{
-	  PurgeComm (get_handle (), PURGE_RXABORT);
-	  tot = -1;
-	  __seterrno ();
+	  if (tot == 0)
+	    {
+	      tot = -1;
+	      __seterrno ();
+	    }
+	  CancelIo (get_handle ());
+	  overlapped_armed = 0;
+	  GetOverlappedResult (get_handle (), &io_status, &undefined, TRUE);
 	  break;
 	}
-
-      n = 0;
     }
+  /* ALL of these are required to loop:
+
+     Still room in user space buffer
+     AND still a minchars requirement (implies blocking mode)
+     AND vtime_ is not set. */
+  while (ulen > 0 && minchars > 0 && vtime_ == 0);
 
 out:
-  ulen = tot;
+  ulen = (size_t) tot;
+  if (is_nonblocking () && ulen == 0)
+    {
+      ulen = (size_t) -1;
+      set_errno (EAGAIN);
+    }
 }
 
 /* Cover function to WriteFile to provide Posix interface and semantics
@@ -595,7 +619,7 @@ fhandler_serial::tcsetattr (int action, const struct termios *t)
   bool dropDTR = false;
   COMMTIMEOUTS to;
   DCB ostate, state;
-  unsigned int ovtime = vtime_, ovmin = vmin_;
+  cc_t ovtime = vtime_, ovmin = vmin_;
   int tmpDtr, tmpRts, res;
   res = tmpDtr = tmpRts = 0;
 
@@ -902,7 +926,7 @@ fhandler_serial::tcsetattr (int action, const struct termios *t)
       vmin_ = t->c_cc[VMIN];
     }
 
-  debug_printf ("vtime %d, vmin %ld", vtime_, vmin_);
+  debug_printf ("vtime %u, vmin %u", vtime_, vmin_);
 
   if (ovmin != vmin_ || ovtime != vtime_)
   {
@@ -1136,7 +1160,7 @@ fhandler_serial::tcgetattr (struct termios *t)
   t->c_cc[VTIME] = vtime_ / 100;
   t->c_cc[VMIN] = vmin_;
 
-  debug_printf ("vmin_ %lu, vtime_ %u", vmin_, vtime_);
+  debug_printf ("vmin_ %u, vtime_ %u", vmin_, vtime_);
 
   return 0;
 }