Re: PATCH /usr/include/X11/Xtrans/Xtranssock.c [WAS: Re: xhost package not compiled for IPv6]

[Jon TURNEY <jon dot turney at dronecode dot org dot uk>]

On 12/08/2009 07:19, cygwin wrote:
> The following patch corrects the problem reported as:

Thanks very much for the patch.

> "The server listens on IPv6 addresses ok and recognizes incoming
> connections, but then the client just hangs. The server logs
> "_XSERVTransSocketINETAccept: accept() failed"
>
>
> --- /usr/include/X11/Xtrans/Xtranssock.c.orig 2009-08-12
> 02:02:43.640625000 -0400
> +++ /usr/include/X11/Xtrans/Xtranssock.c 2009-08-12 02:04:10.078125000
> -0400
> @@ -1255,7 +1255,7 @@
>
> {
> XtransConnInfo newciptr;
> - struct sockaddr_in sockname;
> + struct sockaddr_storage sockname;
> SOCKLEN_T namelen = sizeof(sockname);
>
> PRMSG (2, "SocketINETAccept(%p,%d)\n", ciptr, ciptr->fd, 0);
>
>
> With this in place, and recompiling the X11 server it will properly
> accept client connections over IPv6 transport.

While auditing the rest of libxtrans to see if the same problem occurs 
anywhere else, I notice that this should probably be written as below to avoid 
breaking compilation on IPv4-only systems.

diff --git a/Xtranssock.c b/Xtranssock.c
index 0935744..4d2e2cb 100644
--- a/Xtranssock.c
+++ b/Xtranssock.c
@@ -1260,7 +1260,11 @@ TRANS(SocketINETAccept) (XtransConnInfo ciptr, int *status)

 {
     XtransConnInfo     newciptr;
+#if defined(IPv6) && defined(AF_INET6)
+    struct sockaddr_storage    sockname;
+#else
     struct sockaddr_in sockname;
+#endif
     SOCKLEN_T          namelen = sizeof(sockname);

     PRMSG (2, "SocketINETAccept(%p,%d)\n", ciptr, ciptr->fd, 0);


Hmmm... but if it's really the size of the sockname argument which is causing 
the accept() to fail, this would be a bug in cygwin's accept() implementation, 
as it's supposed to truncate the data written to the sockname, rather than 
fail if it won't fit [1].  If that actually is the case, since we don't 
actually use the peer address here, the code as stands is correct (if a little 
odd).

I suppose I need to write a small test case to look at this...

> The IPv6 socket implementation for the X11 server seems rather poorly
> written, it does not follow the generally recommended practice of
> porting IPv4 code for IPv6/IPv4 dual stack.

Yes, xtrans is recognized as a bit of mess, so well done making any sense out 
of it :-)

[1] http://www.opengroup.org/onlinepubs/009695399/functions/accept.html

--
Jon TURNEY
Volunteer Cygwin/X X Server maintainer

--
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple
Problem reports:       http://cygwin.com/problems.html
Documentation:         http://x.cygwin.com/docs/
FAQ:                   http://x.cygwin.com/docs/faq/