[Qemu-devel] [PATCH] Allow AF_UNIX sockets to be disabled on non-Windows

[Qemu-devel] [PATCH] Allow AF_UNIX sockets to be disabled on non-Windows

[Ian Jackson]

[-- Attachment #1: message body text --]
[-- Type: text/plain, Size: 293 bytes --]

The patch below makes it possible to disable AF_UNIX (unix-domain)
sockets in host environments which do not define _WIN32, by adding
-DNO_UNIX_SOCKETS to the compiler flags.  This is useful in the
effectively-embedded qemu host which are going to be using for device
emulation in Xen.

Ian.


[-- Attachment #2: allow AF_UNIX sockets to be disabled --]
[-- Type: text/plain, Size: 1486 bytes --]

Index: qemu_socket.h
===================================================================
RCS file: /sources/qemu/qemu/qemu_socket.h,v
retrieving revision 1.3
diff -u -r1.3 qemu_socket.h
--- qemu_socket.h	17 Dec 2007 04:42:28 -0000	1.3
+++ qemu_socket.h	25 Jan 2008 14:40:19 -0000
@@ -14,12 +14,19 @@
 #define EINTR       WSAEINTR
 #define EINPROGRESS WSAEINPROGRESS
 
+#ifndef NO_UNIX_SOCKETS
+#define NO_UNIX_SOCKETS 1
+#endif
+
 #else
 
 #include <sys/socket.h>
 #include <netinet/in.h>
 #include <netinet/tcp.h>
+
+#ifndef NO_UNIX_SOCKETS
 #include <sys/un.h>
+#endif
 
 #define socket_error() errno
 #define closesocket(s) close(s)
Index: vl.c
===================================================================
RCS file: /sources/qemu/qemu/vl.c,v
retrieving revision 1.401
diff -u -r1.401 vl.c
--- vl.c	23 Jan 2008 19:01:12 -0000	1.401
+++ vl.c	25 Jan 2008 14:40:23 -0000
@@ -3600,7 +3600,7 @@
     return 0;
 }
 
-#ifndef _WIN32
+#ifndef NO_UNIX_SOCKETS
 static int parse_unix_path(struct sockaddr_un *uaddr, const char *str)
 {
     const char *p;
Index: vnc.c
===================================================================
RCS file: /sources/qemu/qemu/vnc.c,v
retrieving revision 1.33
diff -u -r1.33 vnc.c
--- vnc.c	14 Jan 2008 21:45:55 -0000	1.33
+++ vnc.c	25 Jan 2008 14:40:24 -0000
@@ -2179,7 +2179,7 @@
 	}
 #endif
     }
-#ifndef _WIN32
+#ifndef NO_UNIX_SOCKETS
     if (strstart(display, "unix:", &p)) {
 	addr = (struct sockaddr *)&uaddr;
 	addrlen = sizeof(uaddr);

Re: [Qemu-devel] [PATCH] Allow AF_UNIX sockets to be disabled on non-Windows

[Anthony Liguori]

Ian Jackson wrote:
> The patch below makes it possible to disable AF_UNIX (unix-domain)
> sockets in host environments which do not define _WIN32, by adding
> -DNO_UNIX_SOCKETS to the compiler flags.  This is useful in the
> effectively-embedded qemu host which are going to be using for device
> emulation in Xen.
>
> Ian.
>   

Presumably, this is because you're compiling for MiniOS?  Why not just 
add a _MINIOS define and then add an appropriate #ifndef.

Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH] Allow AF_UNIX sockets to be disabled on non-Windows

[Ian Jackson]

Anthony Liguori writes ("Re: [Qemu-devel] [PATCH] Allow AF_UNIX sockets to be disabled on non-Windows"):
> Presumably, this is because you're compiling for MiniOS?  Why not just 
> add a _MINIOS define and then add an appropriate #ifndef.

Yes.  We'll probably add a MINIOS define, yes (at the moment our tree
is too divergent; these patches are an attempt at restoring some
semblance of sanity to the Xen situation).

But when the existing code says

     }
 #ifndef _WIN32
     if (strstart(display, "unix:", &p)) {
        addr = (struct sockaddr *)&uaddr;
        addrlen = sizeof(uaddr);
     etc.

then changing it to something like

 #if !(defined(_WIN32) || defined(MINIOS)

seems very ugly.

The information about whether a host platform supports AF_UNIX should
be in one place, and there should be one place to turn the
functionality on and off in qemu.  slirp already uses NO_UNIX_SOCKETS
so I just reused that name, although unfortunately it's still set in
two different places.

Ian.

Re: [Qemu-devel] [PATCH] Allow AF_UNIX sockets to be disabled on non-Windows

[Johannes Schindelin]

Hi,

On Fri, 25 Jan 2008, Ian Jackson wrote:

> But when the existing code says
> 
>      }
>  #ifndef _WIN32
>      if (strstart(display, "unix:", &p)) {
>         addr = (struct sockaddr *)&uaddr;
>         addrlen = sizeof(uaddr);
>      etc.
> 
> then changing it to something like
> 
>  #if !(defined(_WIN32) || defined(MINIOS)
> 
> seems very ugly.

Yes, that is very ugly.  But changing it to

#ifndef NO_AF_UNIX_SOCKETS

it actually gives you a bit of documentation what the code does, in 
addition to controlling what is compiled and what not.

Like in the patch we saw today where there were a lot of "#ifdef 
__linux__", it is always good if you can see _why_ some code is enabled or 
disabled, instead of for what platform.

Ciao,
Dscho

Re: [Qemu-devel] [PATCH] Allow AF_UNIX sockets to be disabled on non-Windows

[Ian Jackson]

Johannes Schindelin writes ("Re: [Qemu-devel] [PATCH] Allow AF_UNIX sockets to be disabled on non-Windows"):
> Yes, that is very ugly.  But changing it to
>   #ifndef NO_AF_UNIX_SOCKETS
> it actually gives you a bit of documentation what the code does, in 
> addition to controlling what is compiled and what not.

Indeed so.  That's why I did that :-).

Thanks,
Ian.

[Qemu-devel] Re: [PATCH] Allow AF_UNIX sockets to be disabled on non-Windows

[Ian Jackson]

iwj writes ("[PATCH] Allow AF_UNIX sockets to be disabled on non-Windows"):
> The patch below makes it possible to disable AF_UNIX (unix-domain)
> sockets in host environments which do not define _WIN32, by adding
> -DNO_UNIX_SOCKETS to the compiler flags.  This is useful in the
> effectively-embedded qemu host which are going to be using for device
> emulation in Xen.

If you don't like my patches please do say.

Ian.

Re: [Qemu-devel] Re: [PATCH] Allow AF_UNIX sockets to be disabled on non-Windows

[Anthony Liguori]

Ian Jackson wrote:
> iwj writes ("[PATCH] Allow AF_UNIX sockets to be disabled on non-Windows"):
>   
>> The patch below makes it possible to disable AF_UNIX (unix-domain)
>> sockets in host environments which do not define _WIN32, by adding
>> -DNO_UNIX_SOCKETS to the compiler flags.  This is useful in the
>> effectively-embedded qemu host which are going to be using for device
>> emulation in Xen.
>>     
>
> If you don't like my patches please do say.
>   

It should just check a define for _MINIOS.  That makes it a lot more 
obvious why it's not being included.

Regards,

Anthony Liguori

> Ian.
>
>
>   

Re: [Qemu-devel] Re: [PATCH] Allow AF_UNIX sockets to be disabled on non-Windows

[Samuel Thibault]

Anthony Liguori, le Wed 06 Feb 2008 13:19:23 -0600, a écrit :
> Ian Jackson wrote:
> >iwj writes ("[PATCH] Allow AF_UNIX sockets to be disabled on non-Windows"):
> >  
> >>The patch below makes it possible to disable AF_UNIX (unix-domain)
> >>sockets in host environments which do not define _WIN32, by adding
> >>-DNO_UNIX_SOCKETS to the compiler flags.  This is useful in the
> >>effectively-embedded qemu host which are going to be using for device
> >>emulation in Xen.
> 
> It should just check a define for _MINIOS.

That's exactly what we wanted to avoid.

> That makes it a lot more obvious why it's not being included.

But it doesn't necessarily make obvious _what_ is not being included
(here, local sockets). To my mind, something like

#if !(defined(_WIN32) || defined(_MINIOS))
#define DO_UNIX_SOCKET
#endif

And then in the code, #ifdef DO_UNIX_SOCKET, is much nicer than
repeating the if (!def||def) everywhere (and have to change them all if
another system needs that too)

Samuel