[PATCH] launch: add missing headers on Darwin
Including sys/un.h for sockaddr_un and sys/fcntl.h for O_CLOEXEC/O_NONBLOCK. --- src/launch-libvirt.c | 1 + src/launch-unix.c | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/src/launch-libvirt.c b/src/launch-libvirt.c index 1649884..ad07210 100644 --- a/src/launch-libvirt.c +++ b/src/launch-libvirt.c @@ -52,6 +52,7 @@ /* Fixes for Mac OS X */ #if defined __APPLE__ && defined __MACH__ #include <sys/un.h> +#include <sys/fcntl.h> #endif #ifndef SOCK_CLOEXEC # define SOCK_CLOEXEC O_CLOEXEC diff --git a/src/launch-unix.c b/src/launch-unix.c index 5cce9c1..f57910d 100644 --- a/src/launch-unix.c +++ b/src/launch-unix.c @@ -25,6 +25,10 @@ #include <string.h> #include <libintl.h> +#if defined __APPLE__ && defined __MACH__ +#include <sys/un.h> +#endif + #include "guestfs.h" #include "guestfs-internal.h" #include "guestfs_protocol.h" -- 2.6.2
On Wed, Nov 04, 2015 at 12:05:31PM +0000, Margaret Lewicka wrote:
Including sys/un.h for sockaddr_un and sys/fcntl.h for O_CLOEXEC/O_NONBLOCK. --- src/launch-libvirt.c | 1 + src/launch-unix.c | 4 ++++ 2 files changed, 5 insertions(+)
diff --git a/src/launch-libvirt.c b/src/launch-libvirt.c index 1649884..ad07210 100644 --- a/src/launch-libvirt.c +++ b/src/launch-libvirt.c @@ -52,6 +52,7 @@ /* Fixes for Mac OS X */ #if defined __APPLE__ && defined __MACH__ #include <sys/un.h> +#include <sys/fcntl.h> #endif #ifndef SOCK_CLOEXEC # define SOCK_CLOEXEC O_CLOEXEC diff --git a/src/launch-unix.c b/src/launch-unix.c index 5cce9c1..f57910d 100644 --- a/src/launch-unix.c +++ b/src/launch-unix.c @@ -25,6 +25,10 @@ #include <string.h> #include <libintl.h>
+#if defined __APPLE__ && defined __MACH__ +#include <sys/un.h> +#endif + #include "guestfs.h" #include "guestfs-internal.h" #include "guestfs_protocol.h"
Thanks Margaret, I'll push that shortly. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://people.redhat.com/~rjones/virt-top
On Wednesday 04 November 2015 12:05:31 Margaret Lewicka wrote:
Including sys/un.h for sockaddr_un and sys/fcntl.h for O_CLOEXEC/O_NONBLOCK. --- src/launch-libvirt.c | 1 + src/launch-unix.c | 4 ++++ 2 files changed, 5 insertions(+)
diff --git a/src/launch-libvirt.c b/src/launch-libvirt.c index 1649884..ad07210 100644 --- a/src/launch-libvirt.c +++ b/src/launch-libvirt.c @@ -52,6 +52,7 @@ /* Fixes for Mac OS X */ #if defined __APPLE__ && defined __MACH__ #include <sys/un.h> +#include <sys/fcntl.h> #endif #ifndef SOCK_CLOEXEC # define SOCK_CLOEXEC O_CLOEXEC diff --git a/src/launch-unix.c b/src/launch-unix.c index 5cce9c1..f57910d 100644 --- a/src/launch-unix.c +++ b/src/launch-unix.c @@ -25,6 +25,10 @@ #include <string.h> #include <libintl.h>
+#if defined __APPLE__ && defined __MACH__ +#include <sys/un.h> +#endif + #include "guestfs.h" #include "guestfs-internal.h" #include "guestfs_protocol.h"
NACK, these OS-specific blocks are hard to maintain, and it makes no sense to enclose POSIX headers in them. Can you please try the attached patch? I see Rich already pushed your patch, but IMHO that should be reverted and this one should go... -- Pino Toscano
On 4 November 2015 at 12:37, Pino Toscano <ptoscano@redhat.com> wrote: [...]
NACK, these OS-specific blocks are hard to maintain, and it makes no sense to enclose POSIX headers in them.
Can you please try the attached patch? I see Rich already pushed your patch, but IMHO that should be reverted and this one should go...
Happy to. However, these same header files have been removed in 3b17152ec399acb35b23331d2c3b4f424a68824a: commit 3b17152ec399acb35b23331d2c3b4f424a68824a Author: Richard W.M. Jones <rjones@redhat.com> Date: Tue Sep 29 21:58:41 2015 +0100 lib: Remove unused header files. I used ESR's deheader program to look for unused includes. I then examined each instance by hand, and also test-compiled (on Linux). ...which makes me suspect it will happen again at some point unless there is some indication that it's a non-Linux include. -- Margaret
On Wed, Nov 04, 2015 at 12:43:31PM +0000, Margaret Lewicka wrote:
On 4 November 2015 at 12:37, Pino Toscano <ptoscano@redhat.com> wrote: [...]
NACK, these OS-specific blocks are hard to maintain, and it makes no sense to enclose POSIX headers in them.
Can you please try the attached patch? I see Rich already pushed your patch, but IMHO that should be reverted and this one should go...
Happy to. However, these same header files have been removed in 3b17152ec399acb35b23331d2c3b4f424a68824a:
commit 3b17152ec399acb35b23331d2c3b4f424a68824a Author: Richard W.M. Jones <rjones@redhat.com> Date: Tue Sep 29 21:58:41 2015 +0100
lib: Remove unused header files.
I used ESR's deheader program to look for unused includes. I then examined each instance by hand, and also test-compiled (on Linux).
...which makes me suspect it will happen again at some point unless there is some indication that it's a non-Linux include.
Yup, sorry about that one. Perhaps a comment, like: #include <sys/un.h> /* Required for Mac OS X */ ? Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://people.redhat.com/~rjones/virt-top
On 4 November 2015 at 13:07, Richard W.M. Jones <rjones@redhat.com> wrote: [...]
Happy to. However, these same header files have been removed in 3b17152ec399acb35b23331d2c3b4f424a68824a:
commit 3b17152ec399acb35b23331d2c3b4f424a68824a Author: Richard W.M. Jones <rjones@redhat.com> Date: Tue Sep 29 21:58:41 2015 +0100
lib: Remove unused header files.
I used ESR's deheader program to look for unused includes. I then examined each instance by hand, and also test-compiled (on Linux).
...which makes me suspect it will happen again at some point unless there is some indication that it's a non-Linux include.
Yup, sorry about that one.
Perhaps a comment, like:
#include <sys/un.h> /* Required for Mac OS X */
?
You're the boss(es) -- I'll go with any solution that a) keeps the headers in, b) satisfies your requirements. If a comment is enough to preserve these includes during the next header purge, I'm happy to commit a new patch. -- Margaret
On Wednesday 04 November 2015 12:43:31 Margaret Lewicka wrote:
On 4 November 2015 at 12:37, Pino Toscano <ptoscano@redhat.com> wrote: [...]
NACK, these OS-specific blocks are hard to maintain, and it makes no sense to enclose POSIX headers in them.
Can you please try the attached patch? I see Rich already pushed your patch, but IMHO that should be reverted and this one should go...
Happy to. However, these same header files have been removed in 3b17152ec399acb35b23331d2c3b4f424a68824a:
commit 3b17152ec399acb35b23331d2c3b4f424a68824a Author: Richard W.M. Jones <rjones@redhat.com> Date: Tue Sep 29 21:58:41 2015 +0100
lib: Remove unused header files.
I used ESR's deheader program to look for unused includes. I then examined each instance by hand, and also test-compiled (on Linux).
...which makes me suspect it will happen again at some point unless there is some indication that it's a non-Linux include.
Well, it was because something else implicitly pulled it. Note that sys/un.h is a POSIX header, and it's needed for sockaddr_un [1]. [1] http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/sys_un.h.html -- Pino Toscano
participants (3)
-
Margaret Lewicka -
Pino Toscano -
Richard W.M. Jones