[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] build: improved handling of <execinfo.h>, BSD <net/if.h>



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/


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]