[libvirt] [PATCH] Include some extra headers needed for OpenBSD.
From bafcb4ed2b90b5ba845ca6b61861e3caa548b16a Mon Sep 17 00:00:00 2001 From: Jasper Lievisse Adriaanse <jasper@humppa.nl> Date: Tue, 4 Sep 2012 16:57:09 +0200 Subject: [PATCH] Include some extra headers needed for OpenBSD.
--- src/util/virnetdevbridge.c | 9 +++++++++ 1 files changed, 9 insertions(+), 0 deletions(-) diff --git a/src/util/virnetdevbridge.c b/src/util/virnetdevbridge.c index 7b11bee..8559223 100644 --- a/src/util/virnetdevbridge.c +++ b/src/util/virnetdevbridge.c @@ -30,6 +30,15 @@ #include "intprops.h" #include <sys/ioctl.h> + +#ifdef HAVE_SYS_PARAM_H +# include <sys/param.h> +#endif + +#ifdef HAVE_SYS_SOCKET_H +# include <sys/socket.h> +#endif + #ifdef HAVE_NET_IF_H # include <net/if.h> #endif -- 1.7.6 -- Cheers, Jasper "Stay Hungry. Stay Foolish"
On 09/04/2012 08:57 AM, Jasper Lievisse Adriaanse wrote:
From bafcb4ed2b90b5ba845ca6b61861e3caa548b16a Mon Sep 17 00:00:00 2001 From: Jasper Lievisse Adriaanse <jasper@humppa.nl> Date: Tue, 4 Sep 2012 16:57:09 +0200 Subject: [PATCH] Include some extra headers needed for OpenBSD.
--- src/util/virnetdevbridge.c | 9 +++++++++ 1 files changed, 9 insertions(+), 0 deletions(-)
Please show the compiler errors that you got without these includes. I can't help but wonder if you have instead uncovered a bug in the gnulib headers, but knowing which symbols were not declared makes a difference in answering that question.
diff --git a/src/util/virnetdevbridge.c b/src/util/virnetdevbridge.c index 7b11bee..8559223 100644 --- a/src/util/virnetdevbridge.c +++ b/src/util/virnetdevbridge.c @@ -30,6 +30,15 @@ #include "intprops.h"
#include <sys/ioctl.h> + +#ifdef HAVE_SYS_PARAM_H
<sys/param.h> is non-standard; what are we using that requires us to probe for its existence? Should gnulib consider guaranteeing this header in spite of it being non-standard?
+# include <sys/param.h> +#endif + +#ifdef HAVE_SYS_SOCKET_H
This line shouldn't be necessary; gnulib guarantees a working <sys/socket.h> on all architectures. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Tue, Sep 04, 2012 at 09:18:20AM -0600, Eric Blake wrote:
On 09/04/2012 08:57 AM, Jasper Lievisse Adriaanse wrote:
From bafcb4ed2b90b5ba845ca6b61861e3caa548b16a Mon Sep 17 00:00:00 2001 From: Jasper Lievisse Adriaanse <jasper@humppa.nl> Date: Tue, 4 Sep 2012 16:57:09 +0200 Subject: [PATCH] Include some extra headers needed for OpenBSD.
--- src/util/virnetdevbridge.c | 9 +++++++++ 1 files changed, 9 insertions(+), 0 deletions(-)
Please show the compiler errors that you got without these includes. I can't help but wonder if you have instead uncovered a bug in the gnulib headers, but knowing which symbols were not declared makes a difference in answering that question.
diff --git a/src/util/virnetdevbridge.c b/src/util/virnetdevbridge.c index 7b11bee..8559223 100644 --- a/src/util/virnetdevbridge.c +++ b/src/util/virnetdevbridge.c @@ -30,6 +30,15 @@ #include "intprops.h"
#include <sys/ioctl.h> + +#ifdef HAVE_SYS_PARAM_H
<sys/param.h> is non-standard; what are we using that requires us to probe for its existence? Should gnulib consider guaranteeing this header in spite of it being non-standard? Nope, it was an error on my side to include it. I thought it was needed by socket.h, but after trying to show you the error...there was none.
+# include <sys/param.h> +#endif + +#ifdef HAVE_SYS_SOCKET_H
This line shouldn't be necessary; gnulib guarantees a working <sys/socket.h> on all architectures. OK. Could you please push this one then?
-- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
-- Cheers, Jasper "Stay Hungry. Stay Foolish"
On 09/04/2012 10:23 AM, Jasper Lievisse Adriaanse wrote:
On Tue, Sep 04, 2012 at 09:18:20AM -0600, Eric Blake wrote:
On 09/04/2012 08:57 AM, Jasper Lievisse Adriaanse wrote:
From bafcb4ed2b90b5ba845ca6b61861e3caa548b16a Mon Sep 17 00:00:00 2001 From: Jasper Lievisse Adriaanse <jasper@humppa.nl> Date: Tue, 4 Sep 2012 16:57:09 +0200 Subject: [PATCH] Include some extra headers needed for OpenBSD.
--- src/util/virnetdevbridge.c | 9 +++++++++ 1 files changed, 9 insertions(+), 0 deletions(-)
Please show the compiler errors that you got without these includes. I can't help but wonder if you have instead uncovered a bug in the gnulib headers, but knowing which symbols were not declared makes a difference in answering that question.
I'd still like to know the compiler error you got when <sys/socket.h> was not present, but just guessing from the source code, I see one call of socket() (protected behind #if defined(HAVE_NET_IF_H) && defined(SIOCBRADDBR), but maybe those are both true for OpenBSD?). Even though I'm pushing, I would STILL like to know why.
This line shouldn't be necessary; gnulib guarantees a working <sys/socket.h> on all architectures.
OK. Could you please push this one then?
Here's what I squashed in before pushing; I also added you to AUTHORS. Let me know if you prefer an alternate spelling or email address (the file is in UTF-8, if that matters). diff --git i/src/util/virnetdevbridge.c w/src/util/virnetdevbridge.c index 8559223..a29e4b2 100644 --- i/src/util/virnetdevbridge.c +++ w/src/util/virnetdevbridge.c @@ -30,14 +30,7 @@ #include "intprops.h" #include <sys/ioctl.h> - -#ifdef HAVE_SYS_PARAM_H -# include <sys/param.h> -#endif - -#ifdef HAVE_SYS_SOCKET_H -# include <sys/socket.h> -#endif +#include <sys/socket.h> #ifdef HAVE_NET_IF_H # include <net/if.h> -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Tue, Sep 04, 2012 at 10:45:41AM -0600, Eric Blake wrote:
On 09/04/2012 10:23 AM, Jasper Lievisse Adriaanse wrote:
On Tue, Sep 04, 2012 at 09:18:20AM -0600, Eric Blake wrote:
On 09/04/2012 08:57 AM, Jasper Lievisse Adriaanse wrote:
From bafcb4ed2b90b5ba845ca6b61861e3caa548b16a Mon Sep 17 00:00:00 2001 From: Jasper Lievisse Adriaanse <jasper@humppa.nl> Date: Tue, 4 Sep 2012 16:57:09 +0200 Subject: [PATCH] Include some extra headers needed for OpenBSD.
--- src/util/virnetdevbridge.c | 9 +++++++++ 1 files changed, 9 insertions(+), 0 deletions(-)
Please show the compiler errors that you got without these includes. I can't help but wonder if you have instead uncovered a bug in the gnulib headers, but knowing which symbols were not declared makes a difference in answering that question.
I'd still like to know the compiler error you got when <sys/socket.h> was not present, but just guessing from the source code, I see one call of socket() (protected behind #if defined(HAVE_NET_IF_H) && defined(SIOCBRADDBR), but maybe those are both true for OpenBSD?). Even though I'm pushing, I would STILL like to know why. Of course, here it is:
In file included from util/virnetdevbridge.c:35: /usr/include/net/if.h:276: warning: 'struct sockaddr' declared inside parameter list /usr/include/net/if.h:276: warning: its scope is only this definition or declaration, which is probably not what you want /usr/include/net/if.h:280: warning: 'struct sockaddr' declared inside parameter list /usr/include/net/if.h:293: error: 'AF_MAX' undeclared here (not in a function) /usr/include/net/if.h:606: error: field 'ifru_addr' has incomplete type /usr/include/net/if.h:607: error: field 'ifru_dstaddr' has incomplete type /usr/include/net/if.h:608: error: field 'ifru_broadaddr' has incomplete type /usr/include/net/if.h:626: error: field 'ifra_addr' has incomplete type /usr/include/net/if.h:627: error: field 'ifra_dstaddr' has incomplete type /usr/include/net/if.h:629: error: field 'ifra_mask' has incomplete type /usr/include/net/if.h:673: error: field 'addr' has incomplete type /usr/include/net/if.h:674: error: field 'dstaddr' has incomplete type In file included from /usr/include/net/if.h:692, from util/virnetdevbridge.c:35: /usr/include/net/if_arp.h:79: error: field 'arp_pa' has incomplete type /usr/include/net/if_arp.h:80: error: field 'arp_ha' has incomplete type Error while executing cc -std=gnu99 -DHAVE_CONFIG_H -I. -I.. -I../gnulib/lib -I../gnulib/lib -I../include -I../include -I../src/util -DIN_LIBVIRT -I/usr/local/include -I/usr/local/include/libxml2 -I/usr/local/include -Wall -W -Wformat-y2k -Wformat-security -Winit-self -Wmissing-include-dirs -Wunused -Wunknown-pragmas -Wstrict-aliasing -Wshadow -Wpointer-arith -Wbad-function-cast -Wcast-align -Wwrite-strings -Wstrict-prototypes -Wold-style-definition -Wmissing-noreturn -Wmissing-format-attribute -Wredundant-decls -Wnested-externs -Winline -Winvalid-pch -Wvolatile-register-var -Wdisabled-optimization -Wattributes -Wdeprecated-declarations -Wdiv-by-zero -Wendif-labels -Wextra -Wformat-extra-args -Wformat-zero-length -Wformat=2 -Wmultichar -Wnormalized=nfc -Woverflow -Wpointer-to-int-cast -Wpragmas -Wtrampolines -Wno-missing-field-initializers -Wno-sign-compare -Wno-format-nonliteral -fexceptions -fasynchronous-unwind-tables -fdiagnostics-show-option -funit-at-a-time -fipa-pure-const -I/usr/local/include/dbus-1.0 -I/usr/local/lib/dbus-1.0/include -O2 -pipe -MT libvirt_util_la-virnetdevbridge.lo -MD -MP -MF .deps/libvirt_util_la-virnetdevbridge.Tpo -c util/virnetdevbridge.c -fPIC -DPIC -o .libs/libvirt_util_la-virnetdevbridge.o gmake[3]: *** [libvirt_util_la-virnetdevbridge.lo] Error 1 gmake[3]: Leaving directory `/usr/obj/ports/libvirt-0.10.1/libvirt-0.10.1/src' gmake[2]: *** [all] Error 2 gmake[2]: Leaving directory `/usr/obj/ports/libvirt-0.10.1/libvirt-0.10.1/src' gmake[1]: *** [all-recursive] Error 1 gmake[1]: Leaving directory `/usr/obj/ports/libvirt-0.10.1/libvirt-0.10.1' gmake: *** [all] Error 2
This line shouldn't be necessary; gnulib guarantees a working <sys/socket.h> on all architectures. OK. Could you please push this one then?
Here's what I squashed in before pushing; I also added you to AUTHORS. Let me know if you prefer an alternate spelling or email address (the file is in UTF-8, if that matters). Thanks, my name and e-mail as I sent them are ok.
diff --git i/src/util/virnetdevbridge.c w/src/util/virnetdevbridge.c index 8559223..a29e4b2 100644 --- i/src/util/virnetdevbridge.c +++ w/src/util/virnetdevbridge.c @@ -30,14 +30,7 @@ #include "intprops.h"
#include <sys/ioctl.h> - -#ifdef HAVE_SYS_PARAM_H -# include <sys/param.h> -#endif - -#ifdef HAVE_SYS_SOCKET_H -# include <sys/socket.h> -#endif +#include <sys/socket.h>
#ifdef HAVE_NET_IF_H # include <net/if.h>
-- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
-- Cheers, Jasper "Stay Hungry. Stay Foolish"
[adding gnulib] On 09/04/2012 10:52 AM, Jasper Lievisse Adriaanse wrote:
I'd still like to know the compiler error you got when <sys/socket.h> was not present, but just guessing from the source code, I see one call of socket() (protected behind #if defined(HAVE_NET_IF_H) && defined(SIOCBRADDBR), but maybe those are both true for OpenBSD?). Even though I'm pushing, I would STILL like to know why. Of course, here it is:
In file included from util/virnetdevbridge.c:35: /usr/include/net/if.h:276: warning: 'struct sockaddr' declared inside parameter list
Ouch. The POSIX definition of <net/if.h> doesn't include any interface that needs to use struct sockaddr. Which OpenBSD extension function is triggering this warning? According to POSIX, this .c file should compile: #define _POSIX_C_SOURCE 200809L #include <net/if.h> #include <sys/socket.h> struct if_nameindex i; and it might just compile on OpenBSD (I haven't checked myself); the difference is that we have explicitly asked for namespace pollution beyond what _POSIX_C_SOURCE guarantees, which may explain why 'struct sockaddr' is interfering. But since <net/if.h> is required to be self-contained when in a strict environment, it makes sense for it to also be self-contained in an extension environment. It sounds like gnulib should consider providing a replacement <net/if.h> function to work around this lameness. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Tue, Sep 04, 2012 at 11:08:30AM -0600, Eric Blake wrote:
[adding gnulib]
On 09/04/2012 10:52 AM, Jasper Lievisse Adriaanse wrote:
I'd still like to know the compiler error you got when <sys/socket.h> was not present, but just guessing from the source code, I see one call of socket() (protected behind #if defined(HAVE_NET_IF_H) && defined(SIOCBRADDBR), but maybe those are both true for OpenBSD?). Even though I'm pushing, I would STILL like to know why. Of course, here it is:
In file included from util/virnetdevbridge.c:35: /usr/include/net/if.h:276: warning: 'struct sockaddr' declared inside parameter list
Ouch. The POSIX definition of <net/if.h> doesn't include any interface that needs to use struct sockaddr. Which OpenBSD extension function is triggering this warning? According to POSIX, this .c file should compile:
#define _POSIX_C_SOURCE 200809L #include <net/if.h> #include <sys/socket.h> struct if_nameindex i;
and it might just compile on OpenBSD (I haven't checked myself); the difference is that we have explicitly asked for namespace pollution beyond what _POSIX_C_SOURCE guarantees, which may explain why 'struct sockaddr' is interfering. But since <net/if.h> is required to be self-contained when in a strict environment, it makes sense for it to also be self-contained in an extension environment. It sounds like gnulib should consider providing a replacement <net/if.h> function to work around this lameness.
-- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org That snippet of example code does not compile on OpenBSD:
In file included from foo.c:2: /usr/include/net/if.h:112: error: expected specifier-qualifier-list before 'u_int' /usr/include/net/if.h:125: error: expected specifier-qualifier-list before 'u_char' /usr/include/net/if.h:188: error: expected specifier-qualifier-list before 'u_char' /usr/include/net/if.h:262: error: expected specifier-qualifier-list before 'u_short' /usr/include/net/if.h:474: error: expected specifier-qualifier-list before 'u_int' /usr/include/net/if.h:485: error: expected specifier-qualifier-list before 'u_int' /usr/include/net/if.h:493: error: expected specifier-qualifier-list before 'u_short' /usr/include/net/if.h:512: error: expected specifier-qualifier-list before 'u_short' /usr/include/net/if.h:530: error: expected specifier-qualifier-list before 'u_short' /usr/include/net/if.h:551: error: expected specifier-qualifier-list before 'u_int' /usr/include/net/if.h:586: error: expected specifier-qualifier-list before 'u_int' /usr/include/net/if.h:606: error: field 'ifru_addr' has incomplete type /usr/include/net/if.h:607: error: field 'ifru_dstaddr' has incomplete type /usr/include/net/if.h:608: error: field 'ifru_broadaddr' has incomplete type /usr/include/net/if.h:626: error: field 'ifra_addr' has incomplete type /usr/include/net/if.h:627: error: field 'ifra_dstaddr' has incomplete type /usr/include/net/if.h:629: error: field 'ifra_mask' has incomplete type /usr/include/net/if.h:673: error: field 'addr' has incomplete type /usr/include/net/if.h:674: error: field 'dstaddr' has incomplete type In file included from /usr/include/net/if.h:691, from foo.c:2: /usr/include/net/if_arp.h:79: error: field 'arp_pa' has incomplete type /usr/include/net/if_arp.h:80: error: field 'arp_ha' has incomplete type *** Error code 1 Could you please reference where POSIX states it should, so this can hopefully get fixed in OpenBSD. -- Cheers, Jasper "Stay Hungry. Stay Foolish"
On 09/04/2012 11:23 AM, Jasper Lievisse Adriaanse wrote:
On Tue, Sep 04, 2012 at 11:08:30AM -0600, Eric Blake wrote:
[adding gnulib]
Ouch. The POSIX definition of <net/if.h> doesn't include any interface that needs to use struct sockaddr. Which OpenBSD extension function is triggering this warning? According to POSIX, this .c file should compile:
#define _POSIX_C_SOURCE 200809L #include <net/if.h> #include <sys/socket.h> struct if_nameindex i;
That snippet of example code does not compile on OpenBSD:
In file included from foo.c:2: /usr/include/net/if.h:112: error: expected specifier-qualifier-list before 'u_int'
Could you please reference where POSIX states it should, so this can hopefully get fixed in OpenBSD.
http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/net_if.h.html#tag_1... doesn't mention any prerequisite headers for <net/if.h>. Furthermore, looking at each of the functions in that header, none of them call out a prerequisite: http://pubs.opengroup.org/onlinepubs/9699919799/functions/if_freenameindex.h... http://pubs.opengroup.org/onlinepubs/9699919799/functions/if_indextoname.htm... http://pubs.opengroup.org/onlinepubs/9699919799/functions/if_nameindex.html# http://pubs.opengroup.org/onlinepubs/9699919799/functions/if_nametoindex.htm... If a header cannot be used in isolation, then POSIX would call out multiple headers. For comparison, here's a case where POSIX 2008 _does_ call out multiple headers: http://pubs.opengroup.org/onlinepubs/9699919799/functions/creat.html creat() cannot use the S_IRUSR and other constants for its mode_t argument unless you also include <sys/stat.h>, since <fcntl.h> was not required to be self-contained on that front (there's talk underway about getting that fixed in the next version of POSIX, but that's another story... http://austingroupbugs.net/view.php?id=591) -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Tue, Sep 04, 2012 at 11:37:30AM -0600, Eric Blake wrote:
On 09/04/2012 11:23 AM, Jasper Lievisse Adriaanse wrote:
On Tue, Sep 04, 2012 at 11:08:30AM -0600, Eric Blake wrote:
[adding gnulib]
Ouch. The POSIX definition of <net/if.h> doesn't include any interface that needs to use struct sockaddr. Which OpenBSD extension function is triggering this warning? According to POSIX, this .c file should compile:
#define _POSIX_C_SOURCE 200809L #include <net/if.h> #include <sys/socket.h> struct if_nameindex i;
That snippet of example code does not compile on OpenBSD:
In file included from foo.c:2: /usr/include/net/if.h:112: error: expected specifier-qualifier-list before 'u_int'
Could you please reference where POSIX states it should, so this can hopefully get fixed in OpenBSD.
http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/net_if.h.html#tag_1... doesn't mention any prerequisite headers for <net/if.h>. Furthermore, looking at each of the functions in that header, none of them call out a prerequisite: http://pubs.opengroup.org/onlinepubs/9699919799/functions/if_freenameindex.h... http://pubs.opengroup.org/onlinepubs/9699919799/functions/if_indextoname.htm... http://pubs.opengroup.org/onlinepubs/9699919799/functions/if_nameindex.html# http://pubs.opengroup.org/onlinepubs/9699919799/functions/if_nametoindex.htm...
If a header cannot be used in isolation, then POSIX would call out multiple headers. For comparison, here's a case where POSIX 2008 _does_ call out multiple headers: http://pubs.opengroup.org/onlinepubs/9699919799/functions/creat.html
creat() cannot use the S_IRUSR and other constants for its mode_t argument unless you also include <sys/stat.h>, since <fcntl.h> was not required to be self-contained on that front (there's talk underway about getting that fixed in the next version of POSIX, but that's another story... http://austingroupbugs.net/view.php?id=591)
-- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
A diff for the OpenBSD header file has been created and is currently being tested. So this should be fixed pretty soon (and in time for OpenBSD 5.3). -- Cheers, Jasper "Stay Hungry. Stay Foolish"
On 09/04/2012 11:23 AM, Jasper Lievisse Adriaanse wrote:
On Tue, Sep 04, 2012 at 11:08:30AM -0600, Eric Blake wrote:
[adding gnulib]
Ouch. The POSIX definition of <net/if.h> doesn't include any interface that needs to use struct sockaddr. Which OpenBSD extension function is triggering this warning? According to POSIX, this .c file should compile:
#define _POSIX_C_SOURCE 200809L #include <net/if.h> #include <sys/socket.h> struct if_nameindex i;
It turns out that FreeBSD 8.2 also has a non-self-contained <net/if.h>; so even though I don't have an OpenBSD box handy today, I was able to test a module for a replacement header that works around the issue. Patch coming up shortly. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
FreeBSD and OpenBSD have a <net/if.h> that is not self-contained; and mingw lacks the header altogether. But gnulib has just taken care of that for us, so we might as well simplify our code. In the process, I got a syntax-check failure if we don't also take the gnulib execinfo module. * .gnulib: Update to latest, for execinfo and net_if. * bootstrap.conf (gnulib_modules): Add execinfo and net_if modules. * configure.ac: Let gnulib check for headers. Simplify check for 'struct ifreq', while also including enough prereq headers. * src/internal.h (IF_NAMESIZE): Drop, now that gnulib guarantees it. * src/nwfilter/nwfilter_learnipaddr.h: Use correct header for IF_NAMESIZE. * src/util/virnetdev.c (includes): Assume <net/if.h> exists. * src/util/virnetdevbridge.c (includes): Likewise. * src/util/virnetdevtap.c (includes): Likewise. * src/util/logging.c (includes): Assume <execinfo.h> exists. (virLogStackTraceToFd): Handle gnulib's fallback implementation. --- Successfully tested on Fedora and FreeBSD; I'm still trying to also test a cross-compile to mingw. Gnulib changes amount to: * .gnulib 271dd74...440a1db (36):
net_if: new module readutmp: fix non-portable UT_PID use update from texinfo fts: reduce two or more trailing slashes to just one, usually fts: when there is no risk of overlap, use memcpy, not memmove autoupdate autoupdate manywarnings: update the list of "all" warnings * lib/stdbool.in.h (_Bool) [__cplusplus]: bool, not _Bool. stdbool: be more compatible with mixed C/C++ compiles revert last change: it was not needed tests: test-vc-list-files-git.sh: skip if git is not available gnulib-tool: Remove no-op option --no-changelog. autoupdate doc: remove fdl-1.2.texi execinfo: port to FreeBSD xstrtol.h: avoid "_Noreturn is not at beginning of declaration" warning doc: do not use @acronym stdnoreturn: port to newer GCCs pipe-filter: fix comment typo execinfo: new module extern-inline: support old GCC 'inline' maint.mk: avoid redundant file name in message timer-time: fix link order when static linking on glibc timespec: omit unnecessary AC_C_INLINE stat-time: omit unnecessary AC_C_INLINE ignore-value: omit unnecessary AC_C_INLINE sys_select: avoid 'static inline' mktime: avoid 'static inline' autoupdate gnulib-tool: Improve coding style. gnulib-tool: Fix indentation. gnulib-tool: Remove old file names from .cvsignore, .gitignore. test-parse-datetime: avoid glibc leap-second glitch autoupdate gnulib-tool: Fix indentation of generated gnulib-comp.m4 file.
.gnulib | 2 +- bootstrap.conf | 2 ++ configure.ac | 23 +++++++---------------- src/internal.h | 4 ---- src/nwfilter/nwfilter_learnipaddr.h | 2 ++ src/util/logging.c | 22 +++++++++------------- src/util/virnetdev.c | 4 +--- src/util/virnetdevbridge.c | 6 ++---- src/util/virnetdevtap.c | 4 +--- 9 files changed, 25 insertions(+), 44 deletions(-) diff --git a/.gnulib b/.gnulib index 271dd74..440a1db 160000 --- a/.gnulib +++ b/.gnulib @@ -1 +1 @@ -Subproject commit 271dd74fdf54ec2a03e73a5173b0b5697f6088f1 +Subproject commit 440a1dbe523e37f206252cb034c3a62f26867e42 diff --git a/bootstrap.conf b/bootstrap.conf index 7fefb69..2847c0b 100644 --- a/bootstrap.conf +++ b/bootstrap.conf @@ -38,6 +38,7 @@ count-one-bits crypto/md5 dirname-lgpl environ +execinfo fclose fcntl fcntl-h @@ -70,6 +71,7 @@ manywarnings mkstemp mkstemps mktempd +net_if netdb nonblocking openpty diff --git a/configure.ac b/configure.ac index cb91e7d..47a72b9 100644 --- a/configure.ac +++ b/configure.ac @@ -186,8 +186,7 @@ LIBS=$old_libs dnl Availability of various common headers (non-fatal if missing). AC_CHECK_HEADERS([pwd.h paths.h regex.h sys/un.h \ sys/poll.h syslog.h mntent.h net/ethernet.h linux/magic.h \ - sys/un.h sys/syscall.h netinet/tcp.h ifaddrs.h libtasn1.h \ - net/if.h execinfo.h]) + sys/un.h sys/syscall.h netinet/tcp.h ifaddrs.h libtasn1.h]) dnl We need to decide at configure time if libvirt will use real atomic dnl operations ("lock free") or emulated ones with a mutex. @@ -245,20 +244,12 @@ AM_CONDITIONAL([WITH_ATOMIC_OPS_PTHREAD],[test "$atomic_ops" = "pthread"]) AC_MSG_RESULT([$atomic_ops]) - -AC_MSG_CHECKING([for struct ifreq in net/if.h]) -AC_COMPILE_IFELSE([AC_LANG_PROGRAM( - [[ - #include <net/if.h> - ]], - [[ - struct ifreq ifr; - ]])],[ - AC_DEFINE([HAVE_STRUCT_IFREQ],[],[Defined if struct ifreq existsin net/if.h]) - AC_MSG_RESULT([yes]) - ],[ - AC_MSG_RESULT([yes]) - ]) +AC_CHECK_TYPE([struct ifreq], + [AC_DEFINE([HAVE_STRUCT_IFREQ],[1], + [Defined if struct ifreq exists in net/if.h])], + [], [[#include <sys/socket.h> + #include <net/if.h> + ]]) dnl Our only use of libtasn1.h is in the testsuite, and can be skipped dnl if the header is not present. Assume -ltasn1 is present if the diff --git a/src/internal.h b/src/internal.h index 02fdb8d..8037a4a 100644 --- a/src/internal.h +++ b/src/internal.h @@ -58,10 +58,6 @@ # define HOST_NAME_MAX 256 # endif -# ifndef IF_NAMESIZE -# define IF_NAMESIZE 16 -# endif - # ifndef INET_ADDRSTRLEN # define INET_ADDRSTRLEN 16 # endif diff --git a/src/nwfilter/nwfilter_learnipaddr.h b/src/nwfilter/nwfilter_learnipaddr.h index 977b16d..f2ac85e 100644 --- a/src/nwfilter/nwfilter_learnipaddr.h +++ b/src/nwfilter/nwfilter_learnipaddr.h @@ -2,6 +2,7 @@ * nwfilter_learnipaddr.h: support for learning IP address used by a VM * on an interface * + * Copyright (C) 2012 Red Hat, Inc. * Copyright (C) 2010 IBM Corp. * Copyright (C) 2010 Stefan Berger * @@ -26,6 +27,7 @@ # define __NWFILTER_LEARNIPADDR_H # include "conf/nwfilter_params.h" +# include <net/if.h> enum howDetect { DETECT_DHCP = 1, diff --git a/src/util/logging.c b/src/util/logging.c index a7f6b65..6048151 100644 --- a/src/util/logging.c +++ b/src/util/logging.c @@ -31,12 +31,10 @@ #include <fcntl.h> #include <unistd.h> #include <signal.h> +#include <execinfo.h> #if HAVE_SYSLOG_H # include <syslog.h> #endif -#ifdef HAVE_EXECINFO_H -# include <execinfo.h> -#endif #include "virterror_internal.h" #include "logging.h" @@ -792,23 +790,21 @@ cleanup: static void virLogStackTraceToFd(int fd) { -#ifdef HAVE_EXECINFO_H void *array[100]; int size; - -# define STRIP_DEPTH 3 - - size = backtrace(array, ARRAY_CARDINALITY(array)); - backtrace_symbols_fd(array + STRIP_DEPTH, size - STRIP_DEPTH, fd); - ignore_value(safewrite(fd, "\n", 1)); -#else static bool doneWarning = false; const char *msg = "Stack trace not available on this platform\n"; - if (!doneWarning) { + +#define STRIP_DEPTH 3 + size = backtrace(array, ARRAY_CARDINALITY(array)); + if (size) { + backtrace_symbols_fd(array + STRIP_DEPTH, size - STRIP_DEPTH, fd); + ignore_value(safewrite(fd, "\n", 1)); + } else if (!doneWarning) { ignore_value(safewrite(fd, msg, strlen(msg))); doneWarning = true; } -#endif +#undef STRIP_DEPTH } static int virLogOutputToFd(const char *category ATTRIBUTE_UNUSED, diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index d97820e..f622f5d 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -32,9 +32,7 @@ #include "logging.h" #include <sys/ioctl.h> -#ifdef HAVE_NET_IF_H -# include <net/if.h> -#endif +#include <net/if.h> #include <fcntl.h> #ifdef __linux__ diff --git a/src/util/virnetdevbridge.c b/src/util/virnetdevbridge.c index a29e4b2..4efb98d 100644 --- a/src/util/virnetdevbridge.c +++ b/src/util/virnetdevbridge.c @@ -31,10 +31,8 @@ #include <sys/ioctl.h> #include <sys/socket.h> +#include <net/if.h> -#ifdef HAVE_NET_IF_H -# include <net/if.h> -#endif #ifdef __linux__ # include <linux/sockios.h> # include <linux/param.h> /* HZ */ @@ -47,7 +45,7 @@ #define VIR_FROM_THIS VIR_FROM_NONE -#if defined(HAVE_NET_IF_H) && defined(SIOCBRADDBR) +#ifdef SIOCBRADDBR static int virNetDevSetupControlFull(const char *ifname, struct ifreq *ifr, int domain, diff --git a/src/util/virnetdevtap.c b/src/util/virnetdevtap.c index 24f30b5..37e91d0 100644 --- a/src/util/virnetdevtap.c +++ b/src/util/virnetdevtap.c @@ -35,9 +35,7 @@ #include "util.h" #include <sys/ioctl.h> -#ifdef HAVE_NET_IF_H -# include <net/if.h> -#endif +#include <net/if.h> #include <fcntl.h> #ifdef __linux__ # include <linux/if_tun.h> /* IFF_TUN, IFF_NO_PI */ -- 1.7.11.4
On Wed, Sep 05, 2012 at 04:39:28PM -0600, Eric Blake wrote:
FreeBSD and OpenBSD have a <net/if.h> that is not self-contained; and mingw lacks the header altogether. But gnulib has just taken care of that for us, so we might as well simplify our code. In the process, I got a syntax-check failure if we don't also take the gnulib execinfo module.
* .gnulib: Update to latest, for execinfo and net_if. * bootstrap.conf (gnulib_modules): Add execinfo and net_if modules. * configure.ac: Let gnulib check for headers. Simplify check for 'struct ifreq', while also including enough prereq headers. * src/internal.h (IF_NAMESIZE): Drop, now that gnulib guarantees it. * src/nwfilter/nwfilter_learnipaddr.h: Use correct header for IF_NAMESIZE. * src/util/virnetdev.c (includes): Assume <net/if.h> exists. * src/util/virnetdevbridge.c (includes): Likewise. * src/util/virnetdevtap.c (includes): Likewise. * src/util/logging.c (includes): Assume <execinfo.h> exists. (virLogStackTraceToFd): Handle gnulib's fallback implementation. ---
Successfully tested on Fedora and FreeBSD; I'm still trying to also test a cross-compile to mingw. Gnulib changes amount to:
* .gnulib 271dd74...440a1db (36):
net_if: new module readutmp: fix non-portable UT_PID use update from texinfo fts: reduce two or more trailing slashes to just one, usually fts: when there is no risk of overlap, use memcpy, not memmove autoupdate autoupdate manywarnings: update the list of "all" warnings * lib/stdbool.in.h (_Bool) [__cplusplus]: bool, not _Bool. stdbool: be more compatible with mixed C/C++ compiles revert last change: it was not needed tests: test-vc-list-files-git.sh: skip if git is not available gnulib-tool: Remove no-op option --no-changelog. autoupdate doc: remove fdl-1.2.texi execinfo: port to FreeBSD xstrtol.h: avoid "_Noreturn is not at beginning of declaration" warning doc: do not use @acronym stdnoreturn: port to newer GCCs pipe-filter: fix comment typo execinfo: new module extern-inline: support old GCC 'inline' maint.mk: avoid redundant file name in message timer-time: fix link order when static linking on glibc timespec: omit unnecessary AC_C_INLINE stat-time: omit unnecessary AC_C_INLINE ignore-value: omit unnecessary AC_C_INLINE sys_select: avoid 'static inline' mktime: avoid 'static inline' autoupdate gnulib-tool: Improve coding style. gnulib-tool: Fix indentation. gnulib-tool: Remove old file names from .cvsignore, .gitignore. test-parse-datetime: avoid glibc leap-second glitch autoupdate gnulib-tool: Fix indentation of generated gnulib-comp.m4 file.
.gnulib | 2 +- bootstrap.conf | 2 ++ configure.ac | 23 +++++++---------------- src/internal.h | 4 ---- src/nwfilter/nwfilter_learnipaddr.h | 2 ++ src/util/logging.c | 22 +++++++++------------- src/util/virnetdev.c | 4 +--- src/util/virnetdevbridge.c | 6 ++---- src/util/virnetdevtap.c | 4 +--- 9 files changed, 25 insertions(+), 44 deletions(-)
diff --git a/.gnulib b/.gnulib index 271dd74..440a1db 160000 --- a/.gnulib +++ b/.gnulib @@ -1 +1 @@ -Subproject commit 271dd74fdf54ec2a03e73a5173b0b5697f6088f1 +Subproject commit 440a1dbe523e37f206252cb034c3a62f26867e42 diff --git a/bootstrap.conf b/bootstrap.conf index 7fefb69..2847c0b 100644 --- a/bootstrap.conf +++ b/bootstrap.conf @@ -38,6 +38,7 @@ count-one-bits crypto/md5 dirname-lgpl environ +execinfo fclose fcntl fcntl-h @@ -70,6 +71,7 @@ manywarnings mkstemp mkstemps mktempd +net_if netdb nonblocking openpty diff --git a/configure.ac b/configure.ac index cb91e7d..47a72b9 100644 --- a/configure.ac +++ b/configure.ac @@ -186,8 +186,7 @@ LIBS=$old_libs dnl Availability of various common headers (non-fatal if missing). AC_CHECK_HEADERS([pwd.h paths.h regex.h sys/un.h \ sys/poll.h syslog.h mntent.h net/ethernet.h linux/magic.h \ - sys/un.h sys/syscall.h netinet/tcp.h ifaddrs.h libtasn1.h \ - net/if.h execinfo.h]) + sys/un.h sys/syscall.h netinet/tcp.h ifaddrs.h libtasn1.h])
dnl We need to decide at configure time if libvirt will use real atomic dnl operations ("lock free") or emulated ones with a mutex. @@ -245,20 +244,12 @@ AM_CONDITIONAL([WITH_ATOMIC_OPS_PTHREAD],[test "$atomic_ops" = "pthread"]) AC_MSG_RESULT([$atomic_ops])
- -AC_MSG_CHECKING([for struct ifreq in net/if.h]) -AC_COMPILE_IFELSE([AC_LANG_PROGRAM( - [[ - #include <net/if.h> - ]], - [[ - struct ifreq ifr; - ]])],[ - AC_DEFINE([HAVE_STRUCT_IFREQ],[],[Defined if struct ifreq existsin net/if.h]) - AC_MSG_RESULT([yes]) - ],[ - AC_MSG_RESULT([yes]) - ]) +AC_CHECK_TYPE([struct ifreq], + [AC_DEFINE([HAVE_STRUCT_IFREQ],[1], + [Defined if struct ifreq exists in net/if.h])], + [], [[#include <sys/socket.h> + #include <net/if.h> + ]])
dnl Our only use of libtasn1.h is in the testsuite, and can be skipped dnl if the header is not present. Assume -ltasn1 is present if the diff --git a/src/internal.h b/src/internal.h index 02fdb8d..8037a4a 100644 --- a/src/internal.h +++ b/src/internal.h @@ -58,10 +58,6 @@ # define HOST_NAME_MAX 256 # endif
-# ifndef IF_NAMESIZE -# define IF_NAMESIZE 16 -# endif - # ifndef INET_ADDRSTRLEN # define INET_ADDRSTRLEN 16 # endif diff --git a/src/nwfilter/nwfilter_learnipaddr.h b/src/nwfilter/nwfilter_learnipaddr.h index 977b16d..f2ac85e 100644 --- a/src/nwfilter/nwfilter_learnipaddr.h +++ b/src/nwfilter/nwfilter_learnipaddr.h @@ -2,6 +2,7 @@ * nwfilter_learnipaddr.h: support for learning IP address used by a VM * on an interface * + * Copyright (C) 2012 Red Hat, Inc. * Copyright (C) 2010 IBM Corp. * Copyright (C) 2010 Stefan Berger * @@ -26,6 +27,7 @@ # define __NWFILTER_LEARNIPADDR_H
# include "conf/nwfilter_params.h" +# include <net/if.h>
enum howDetect { DETECT_DHCP = 1, diff --git a/src/util/logging.c b/src/util/logging.c index a7f6b65..6048151 100644 --- a/src/util/logging.c +++ b/src/util/logging.c @@ -31,12 +31,10 @@ #include <fcntl.h> #include <unistd.h> #include <signal.h> +#include <execinfo.h> #if HAVE_SYSLOG_H # include <syslog.h> #endif -#ifdef HAVE_EXECINFO_H -# include <execinfo.h> -#endif
#include "virterror_internal.h" #include "logging.h" @@ -792,23 +790,21 @@ cleanup:
static void virLogStackTraceToFd(int fd) { -#ifdef HAVE_EXECINFO_H void *array[100]; int size; - -# define STRIP_DEPTH 3 - - size = backtrace(array, ARRAY_CARDINALITY(array)); - backtrace_symbols_fd(array + STRIP_DEPTH, size - STRIP_DEPTH, fd); - ignore_value(safewrite(fd, "\n", 1)); -#else static bool doneWarning = false; const char *msg = "Stack trace not available on this platform\n"; - if (!doneWarning) { + +#define STRIP_DEPTH 3 + size = backtrace(array, ARRAY_CARDINALITY(array)); + if (size) { + backtrace_symbols_fd(array + STRIP_DEPTH, size - STRIP_DEPTH, fd); + ignore_value(safewrite(fd, "\n", 1)); + } else if (!doneWarning) { ignore_value(safewrite(fd, msg, strlen(msg))); doneWarning = true; } -#endif +#undef STRIP_DEPTH }
static int virLogOutputToFd(const char *category ATTRIBUTE_UNUSED, diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index d97820e..f622f5d 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -32,9 +32,7 @@ #include "logging.h"
#include <sys/ioctl.h> -#ifdef HAVE_NET_IF_H -# include <net/if.h> -#endif +#include <net/if.h> #include <fcntl.h>
#ifdef __linux__ diff --git a/src/util/virnetdevbridge.c b/src/util/virnetdevbridge.c index a29e4b2..4efb98d 100644 --- a/src/util/virnetdevbridge.c +++ b/src/util/virnetdevbridge.c @@ -31,10 +31,8 @@
#include <sys/ioctl.h> #include <sys/socket.h> +#include <net/if.h>
-#ifdef HAVE_NET_IF_H -# include <net/if.h> -#endif #ifdef __linux__ # include <linux/sockios.h> # include <linux/param.h> /* HZ */ @@ -47,7 +45,7 @@ #define VIR_FROM_THIS VIR_FROM_NONE
-#if defined(HAVE_NET_IF_H) && defined(SIOCBRADDBR) +#ifdef SIOCBRADDBR static int virNetDevSetupControlFull(const char *ifname, struct ifreq *ifr, int domain, diff --git a/src/util/virnetdevtap.c b/src/util/virnetdevtap.c index 24f30b5..37e91d0 100644 --- a/src/util/virnetdevtap.c +++ b/src/util/virnetdevtap.c @@ -35,9 +35,7 @@ #include "util.h"
#include <sys/ioctl.h> -#ifdef HAVE_NET_IF_H -# include <net/if.h> -#endif +#include <net/if.h> #include <fcntl.h> #ifdef __linux__ # include <linux/if_tun.h> /* IFF_TUN, IFF_NO_PI */
ACK, let's push that sooner than later :-) Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
On 09/06/2012 01:41 AM, Daniel Veillard wrote:
On Wed, Sep 05, 2012 at 04:39:28PM -0600, Eric Blake wrote:
FreeBSD and OpenBSD have a <net/if.h> that is not self-contained; and mingw lacks the header altogether. But gnulib has just taken care of that for us, so we might as well simplify our code. In the process, I got a syntax-check failure if we don't also take the gnulib execinfo module.
ACK, let's push that sooner than later :-)
Thanks; pushed. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (3)
-
Daniel Veillard -
Eric Blake -
Jasper Lievisse Adriaanse