Subject: Cygwin Winsock recv/WSARecv(..., MSG_PEEK) patch (Take 3)
From: Jason Tishler <[email-masked]>
To: [email-masked]
Cc: Cygwin <[email-masked]>
Date: Thu, 06 Jun 2002 16:58:09 -0400

Eric,

On Thu, Jun 06, 2002 at 11:18:36AM -0400, Eric S Raymond wrote:
> Jason Tishler <[email-masked]>:
> > Nevertheless, thanks for your patch.  Yours is "better" because it removes
> > the code duplicate between the SSL and non-SSL cases.  My second version
> > is "better" because it minimizes perturbing the existing code base.
> > 
> > Maybe a combination of the two would be best?
> > 
> >     o elimination of code duplication (from yours)
> >     o use of cygwin_read (via #def fm_read) to hide the Cygwin Winsock
> >       workaround (from mine)
> > 
> > Eric,
> > 
> > Do you have any opinions on what is the best approach?  Also, would
> > you consider one of these patches for inclusion in 6.0.0 (or 5.9.13)?
> > Or, should I resubmit after 6.0.0?
> 
> This sounds like it qualifies as a bug fix.  I'll cheerfully take a combined
> patch for 6.0.0.

Attached is the above mentioned combined patch (against fetchmail
5.9.12).  Specifically, it is composed of Sunil Shetye's eliminate
the duplicate SockRead() code patch:

    http://lists.ccil.org/pipermail/fetchmail-friends/2002-June/002213.html

and my hide the Cygwin Winsock workaround patch:

    http://cygwin.com/ml/cygwin/2002-05/msg01694.html

Thanks,
Jason
--- fetchmail-5.9.12.orig/socket.c	2002-05-24 03:27:06.000000000 -0400
+++ fetchmail-5.9.12/socket.c	2002-06-06 12:28:14.000000000 -0400
@@ -40,7 +40,7 @@
 #include "fetchmail.h"
 #include "i18n.h"
 
-/* Defines to allow BeOS to play nice... */
+/* Defines to allow BeOS and Cygwin to play nice... */
 #ifdef __BEOS__
 static char peeked;
 #define fm_close(a)  closesocket(a)
@@ -51,7 +51,12 @@ static char peeked;
 #define fm_close(a)  close(a)
 #define fm_write(a,b,c)  write(a,b,c)
 #define fm_peek(a,b,c)   recv(a,b,c, MSG_PEEK)
+#ifdef __CYGWIN__
+#define fm_read(a,b,c)   cygwin_read(a,b,c)
+static ssize_t cygwin_read(int sock, void *buf, size_t count);
+#else /* ! __CYGWIN__ */
 #define fm_read(a,b,c)   read(a,b,c)
+#endif /* __CYGWIN__ */
 #endif
 
 /* We need to define h_errno only if it is not already */
@@ -536,7 +541,7 @@ int SockWrite(int sock, char *buf, int l
 int SockRead(int sock, char *buf, int len)
 {
     char *newline, *bp = buf;
-    int n, n2;
+    int n;
 #ifdef	SSL_ENABLE
     SSL *ssl;
 #endif
@@ -608,46 +613,24 @@ int SockRead(int sock, char *buf, int le
 				out of the loop now */
 			newline = bp;
 		}
-	} else {
-		if ((n = fm_peek(sock, bp, len)) <= 0)
-			return(-1);
-		if ((newline = memchr(bp, '\n', n)) != NULL)
-			n = newline - bp + 1;
-		if ((n = fm_read(sock, bp, n)) == -1)
-			return(-1);
 	}
-#else
+	else
+#endif /* SSL_ENABLE */
+	{
 
 #ifdef __BEOS__
-    if ((n = fm_read(sock, bp, 1)) <= 0)
+	    if ((n = fm_read(sock, bp, 1)) <= 0)
 #else
-    if ((n = fm_peek(sock, bp, len)) <= 0)
+	    if ((n = fm_peek(sock, bp, len)) <= 0)
 #endif
-        return (-1);
-	if ((newline = memchr(bp, '\n', n)) != NULL)
-	    n = newline - bp + 1;
+		return (-1);
+	    if ((newline = memchr(bp, '\n', n)) != NULL)
+		n = newline - bp + 1;
 #ifndef __BEOS__
-	if ((n2 = fm_read(sock, bp, n)) == -1)
-	    return(-1);
-#ifdef __CYGWIN__
-	/*
-	 * Workaround Microsoft Winsock recv/WSARecv(..., MSG_PEEK) bug.
-	 * See http://sources.redhat.com/ml/cygwin/2001-08/msg00628.html
-	 * for more details.
-	 */
-	if (n2 != n) {
-	    int n3;
-	    if (outlevel >= O_VERBOSE)
-		report(stdout, GT_("Cygwin socket read retry\n"));
-	    n3 = fm_read(sock, bp + n2, n - n2);
-	    if (n3 == -1 || n2 + n3 != n) {
-		report(stderr, GT_("Cygwin socket read retry failed!\n"));
+	    if ((n = fm_read(sock, bp, n)) == -1)
 		return(-1);
-	    }
-	}
-#endif /* __CYGWIN__ */
 #endif /* __BEOS__ */
-#endif
+	}
 	bp += n;
 	len -= n;
     } while 
@@ -1015,6 +998,33 @@ int SockClose(int sock)
     return(fm_close(sock));	/* this is guarded */
 }
 
+#ifdef __CYGWIN__
+/*
+ * Workaround Microsoft Winsock recv/WSARecv(..., MSG_PEEK) bug.
+ * See http://sources.redhat.com/ml/cygwin/2001-08/msg00628.html
+ * for more details.
+ */
+static ssize_t cygwin_read(int sock, void *buf, size_t count)
+{
+    char *bp = buf;
+    int n = 0;
+
+    if ((n = read(sock, bp, count)) == -1)
+	return(-1);
+
+    if (n != count) {
+	int n2 = 0;
+	if (outlevel >= O_VERBOSE)
+	    report(stdout, GT_("Cygwin socket read retry\n"));
+	n2 = read(sock, bp + n, count - n);
+	if (n2 == -1 || n + n2 != count) {
+	    report(stderr, GT_("Cygwin socket read retry failed!\n"));
+	    return(-1);
+	}
+    }
+}
+#endif /* __CYGWIN__ */
+
 #ifdef MAIN
 /*
  * Use the chargen service to test input buffering directly.

--
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple
Bug reporting:         http://cygwin.com/bugs.html
Documentation:         http://cygwin.com/docs.html
FAQ:                   http://cygwin.com/faq/