Re: windows sockets vs. file descriptors bugs in Guile

[Neil Jerram <neil@ossau.uklinux.net>]

[-- Attachment #1: Type: text/plain, Size: 1818 bytes --]

Scott McPeak <smcpeak@coverity.com> writes:

> Hi,
>
> Guile-1.8.7 appears to have socket support and Windows support, but
> they don't seem to work together, despite the existence of things like
> win32-socket.c.

I hit this too about two years ago, but never checked in my patches.
Sorry!

>  (I'm building Guile for Windows using the mingw cross
> compiler on linux.

I was building on Windows with MSVC at the time - which I believe is
quite similar to mingw cross compiling.

>  Maybe it's different with Cygwin?)

Yes, Cygwin has a thicker emulation layer underneath what Guile thinks
is the C library API.

> Specifically, Guile assumes the POSIX rule that a socket is just a
> file descriptor, but on Windows that does not work; socket functions
> only accept sockets, and file functions only accept file descriptors.
> Several Guile functions are affected; I don't think this is an
> exhaustive list, but it's what I ran into while trying to get a simple
> client and server working (see testcase below):
>
> * fport_fill_input: Passes a socket to 'read'.
>
> * write_all: Passes a socket to 'write'.
>
> * fport_close: Passes a socket to 'close'.  The EBADF error message is
> then silently discarded (...), but the bug still manifests, e.g., as a
> server that never closes its connections.

I've attached my patch for these.  It's a bit simpler than yours, and
also avoids needing copyright assignment from you.  Can you take a look,
see if you notice any disadvantages compared with your version, and if
possible try it out?

> * scm_std_select: Passes a pipe file descriptor to 'select'.

I have no record of hitting this one; I suspect my code at the time just
didn't use `sleep'.  I'm wondering if we still need scm_std_select in
Guile now anyway.  I'll write again about that.

Regards,
        Neil


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Use-socket-specific-operations-for-socket-ports-on-W.patch --]
[-- Type: text/x-diff, Size: 3186 bytes --]

From ea9af46454f044bf01094ea15b3eb0a58717bf58 Mon Sep 17 00:00:00 2001
From: Neil Jerram <neil@ossau.uklinux.net>
Date: Fri, 2 Oct 2009 23:13:05 +0100
Subject: [PATCH] Use socket-specific operations for socket ports on Windows

* libguile/socket.c (sym_socket): Made global, for use in identifying
  socket ports in other files.

* libguile/fports.c (fport_fill_input): If port is a socket, and
  #ifdef __MINGW32__, use recv to read from it instead of read.
  (write_all, fport_flush): Similarly, use send instead of write.
  (fport_close): Similarly, use closesocket instead of close.
---
 libguile/fports.c |   34 ++++++++++++++++++++++++++++++----
 libguile/socket.c |    2 +-
 2 files changed, 31 insertions(+), 5 deletions(-)

diff --git a/libguile/fports.c b/libguile/fports.c
index 5d37495..4bc5075 100644
--- a/libguile/fports.c
+++ b/libguile/fports.c
@@ -58,6 +58,7 @@
 #ifdef __MINGW32__
 # include <sys/stat.h>
 # include <winsock2.h>
+extern SCM sym_socket;
 #endif /* __MINGW32__ */
 
 #include <full-write.h>
@@ -604,7 +605,14 @@ fport_fill_input (SCM port)
 #ifndef __MINGW32__
   fport_wait_for_input (port);
 #endif /* !__MINGW32__ */
-  SCM_SYSCALL (count = read (fp->fdes, pt->read_buf, pt->read_buf_size));
+
+#ifdef __MINGW32__
+  if (scm_is_eq (SCM_FILENAME (port), sym_socket))
+    SCM_SYSCALL (count = recv (fp->fdes, pt->read_buf, pt->read_buf_size, 0));
+  else
+#endif
+    SCM_SYSCALL (count = read (fp->fdes, pt->read_buf, pt->read_buf_size));
+
   if (count == -1)
     scm_syserror ("fport_fill_input");
   if (count == 0)
@@ -689,7 +697,12 @@ static void write_all (SCM port, const void *data, size_t remaining)
     {
       size_t done;
 
-      SCM_SYSCALL (done = write (fdes, data, remaining));
+#ifdef __MINGW32__
+      if (scm_is_eq (SCM_FILENAME (port), sym_socket))
+	SCM_SYSCALL (done = send (fdes, data, remaining, 0));
+      else
+#endif
+	SCM_SYSCALL (done = write (fdes, data, remaining));
 
       if (done == -1)
 	SCM_SYSERROR;
@@ -774,7 +787,13 @@ fport_flush (SCM port)
     {
       long count;
 
-      SCM_SYSCALL (count = write (fp->fdes, ptr, remaining));
+#ifdef __MINGW32__
+      if (scm_is_eq (SCM_FILENAME (port), sym_socket))
+	SCM_SYSCALL (count = send (fp->fdes, ptr, remaining, 0));
+      else
+#endif
+	SCM_SYSCALL (count = write (fp->fdes, ptr, remaining));
+
       if (count < 0)
 	{
 	  /* error.  assume nothing was written this call, but
@@ -846,7 +865,14 @@ fport_close (SCM port)
   int rv;
 
   fport_flush (port);
-  SCM_SYSCALL (rv = close (fp->fdes));
+
+#ifdef __MINGW32__
+  if (scm_is_eq (SCM_FILENAME (port), sym_socket))
+    SCM_SYSCALL (rv = closesocket (fp->fdes));
+  else
+#endif
+    SCM_SYSCALL (rv = close (fp->fdes));
+
   if (rv == -1 && errno != EBADF)
     {
       if (scm_gc_running_p)
diff --git a/libguile/socket.c b/libguile/socket.c
index ea2ba5c..2cbb7ce 100644
--- a/libguile/socket.c
+++ b/libguile/socket.c
@@ -439,7 +439,7 @@ SCM_DEFINE (scm_inet_ntop, "inet-ntop", 2, 0, 0,
 
 #endif  /* HAVE_IPV6 */
 
-SCM_SYMBOL (sym_socket, "socket");
+SCM_GLOBAL_SYMBOL (sym_socket, "socket");
 
 #define SCM_SOCK_FD_TO_PORT(fd) scm_fdes_to_port (fd, "r+0", sym_socket)
 
-- 
1.5.6.5