[libvirt] [PATCH 0/5] build: rely more on gnulib
I can't yet upgrade to the latest gnulib due to some LGPLv2+ issues being worked out upstream. But in the meantime, here's some cleanups that will make it easier to upgrade when gnulib is ready. I always like patch series with a net reduction in lines. I suppose I should try to get a mingw or cygwin build finished with these in place, before pushing anything, but that will have to be tomorrow (I tested on Linux, but it's late for me now). bootstrap.conf | 3 +++ configure.ac | 39 +++++++++++++-------------------------- src/Makefile.am | 3 ++- src/libvirt.c | 4 +--- src/nodeinfo.c | 12 ++---------- src/remote/remote_driver.c | 11 ++++------- src/storage/storage_backend.c | 4 +--- src/util/ebtables.c | 7 ++----- src/util/hooks.c | 4 +--- src/util/iptables.c | 7 ++----- src/util/threads.c | 4 ++-- src/util/threads.h | 4 ++-- src/util/util.c | 14 ++++++-------- 13 files changed, 41 insertions(+), 75 deletions(-) [PATCH 1/5] build: rely on gnulib's pthread module [PATCH 2/5] build: replace redundant header check with function check [PATCH 3/5] build: use gnulib's uname [PATCH 4/5] build: use gnulib's sys/wait.h [PATCH 5/5] build: drop more redundant configure checks
No need to repeat the work already provided by gnulib. * bootstrap.conf (gnulib_modules): Add pthread. * configure.ac: Drop all pthread.h checks. * src/Makefile.am (libvirt_lxc_LDADD): Ensure proper link. --- bootstrap.conf | 1 + configure.ac | 8 -------- src/Makefile.am | 3 ++- 3 files changed, 3 insertions(+), 9 deletions(-) diff --git a/bootstrap.conf b/bootstrap.conf index 785489b..da7cc9c 100644 --- a/bootstrap.conf +++ b/bootstrap.conf @@ -43,6 +43,7 @@ perror physmem poll posix-shell +pthread recv random_r send diff --git a/configure.ac b/configure.ac index f883ef7..cfefc02 100644 --- a/configure.ac +++ b/configure.ac @@ -138,14 +138,6 @@ AM_CONDITIONAL([HAVE_GLIBC_RPCGEN], [test "x$ac_cv_path_RPCGEN" != "xno" && $ac_cv_path_RPCGEN -t </dev/null >/dev/null 2>&1]) -dnl pthread? -AC_CHECK_HEADER([pthread.h], - [AC_CHECK_LIB([pthread],[pthread_join],[ - AC_DEFINE([HAVE_LIBPTHREAD],[],[Define if pthread (-lpthread)]) - AC_DEFINE([HAVE_PTHREAD_H],[],[Define if <pthread.h>]) - LIBS="-lpthread $LIBS" - ])]) - dnl Miscellaneous external programs. AC_PATH_PROG([RM], [rm], [/bin/rm]) AC_PATH_PROG([MV], [mv], [/bin/mv]) diff --git a/src/Makefile.am b/src/Makefile.am index d8466f0..b14b63a 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -984,7 +984,8 @@ libvirt_lxc_SOURCES = \ $(CPU_CONF_SOURCES) \ $(NWFILTER_PARAM_CONF_SOURCES) libvirt_lxc_LDFLAGS = $(WARN_CFLAGS) $(COVERAGE_LDCFLAGS) $(CAPNG_LIBS) $(YAJL_LIBS) -libvirt_lxc_LDADD = $(LIBXML_LIBS) $(NUMACTL_LIBS) ../gnulib/lib/libgnu.la +libvirt_lxc_LDADD = $(LIBXML_LIBS) $(NUMACTL_LIBS) $(LIB_PTHREAD) \ + ../gnulib/lib/libgnu.la libvirt_lxc_CFLAGS = \ $(LIBPARTED_CFLAGS) \ $(NUMACTL_CFLAGS) \ -- 1.6.6.1
Eric Blake wrote:
No need to repeat the work already provided by gnulib.
* bootstrap.conf (gnulib_modules): Add pthread. * configure.ac: Drop all pthread.h checks. * src/Makefile.am (libvirt_lxc_LDADD): Ensure proper link.
ACK.
Gnulib guarantees that pthread.h exists, but for now, it is a dummy header with no support for most pthread_* functions. Modify our use of pthread to use function checks, rather than header checks, to determine how much pthread support is present. * configure.ac: Optimize function checks. Add check for pthread functions. * src/remote/remote_driver.c (remoteIOEventLoop): Depend on pthread_sigmask, now that gnulib guarantees pthread.h. * src/util/util.c (virFork): Likewise. * src/util/threads.c (threads-pthread.c): Depend on pthread_mutexattr_init, as a witness of full pthread support. * src/util/threads.h (threads-pthread.h): Likewise. --- configure.ac | 13 +++++++++++-- src/remote/remote_driver.c | 6 +++--- src/util/threads.c | 4 ++-- src/util/threads.h | 4 ++-- src/util/util.c | 10 +++++----- 5 files changed, 23 insertions(+), 14 deletions(-) diff --git a/configure.ac b/configure.ac index cfefc02..b5ff348 100644 --- a/configure.ac +++ b/configure.ac @@ -107,10 +107,19 @@ dnl Use --disable-largefile if you don't want this. AC_SYS_LARGEFILE dnl Availability of various common functions (non-fatal if missing). -AC_CHECK_FUNCS([cfmakeraw regexec uname sched_getaffinity getuid getgid posix_fallocate mmap]) +AC_CHECK_FUNCS_ONCE([cfmakeraw regexec uname sched_getaffinity getuid getgid \ + posix_fallocate mmap]) dnl Availability of various not common threadsafe functions -AC_CHECK_FUNCS([strerror_r strtok_r getmntent_r getgrnam_r getpwuid_r]) +AC_CHECK_FUNCS_ONCE([strerror_r strtok_r getmntent_r getgrnam_r getpwuid_r]) + +dnl Availability of pthread functions (if missing, win32 threading is +dnl assumed). Because of $LIB_PTHREAD, we cannot use AC_CHECK_FUNCS_ONCE. +dnl LIB_PTHREAD was set during gl_INIT by gnulib. +old_LIBS=$LIBS +LIBS="$LIBS $LIB_PTHREAD" +AC_CHECK_FUNCS([pthread_sigmask pthread_mutexattr_init]) +LIBS=$old_libs dnl Availability of various common headers (non-fatal if missing). AC_CHECK_HEADERS([pwd.h paths.h regex.h sys/syslimits.h sys/utsname.h \ diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 1917f26..b798cdf 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -9487,7 +9487,7 @@ remoteIOEventLoop(virConnectPtr conn, struct remote_thread_call *tmp = priv->waitDispatch; struct remote_thread_call *prev; char ignore; -#ifdef HAVE_PTHREAD_H +#ifdef HAVE_PTHREAD_SIGMASK sigset_t oldmask, blockedsigs; #endif @@ -9516,7 +9516,7 @@ remoteIOEventLoop(virConnectPtr conn, * after the call (RHBZ#567931). Same for SIGCHLD and SIGPIPE * at the suggestion of Paolo Bonzini and Daniel Berrange. */ -#ifdef HAVE_PTHREAD_H +#ifdef HAVE_PTHREAD_SIGMASK sigemptyset (&blockedsigs); sigaddset (&blockedsigs, SIGWINCH); sigaddset (&blockedsigs, SIGCHLD); @@ -9529,7 +9529,7 @@ remoteIOEventLoop(virConnectPtr conn, if (ret < 0 && errno == EAGAIN) goto repoll; -#ifdef HAVE_PTHREAD_H +#ifdef HAVE_PTHREAD_SIGMASK ignore_value(pthread_sigmask(SIG_SETMASK, &oldmask, NULL)); #endif diff --git a/src/util/threads.c b/src/util/threads.c index 18f8b64..8c0a91a 100644 --- a/src/util/threads.c +++ b/src/util/threads.c @@ -1,7 +1,7 @@ /* * threads.c: basic thread synchronization primitives * - * Copyright (C) 2009 Red Hat, Inc. + * Copyright (C) 2009-2010 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -23,7 +23,7 @@ #include "threads.h" -#ifdef HAVE_PTHREAD_H +#ifdef HAVE_PTHREAD_MUTEXATTR_INIT # include "threads-pthread.c" #else # ifdef WIN32 diff --git a/src/util/threads.h b/src/util/threads.h index 6e01082..8b2be8d 100644 --- a/src/util/threads.h +++ b/src/util/threads.h @@ -1,7 +1,7 @@ /* * threads.h: basic thread synchronization primitives * - * Copyright (C) 2009 Red Hat, Inc. + * Copyright (C) 2009-2010 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -61,7 +61,7 @@ int virThreadLocalInit(virThreadLocalPtr l, void *virThreadLocalGet(virThreadLocalPtr l); void virThreadLocalSet(virThreadLocalPtr l, void*); -# ifdef HAVE_PTHREAD_H +# ifdef HAVE_PTHREAD_MUTEXATTR_INIT # include "threads-pthread.h" # else # ifdef WIN32 diff --git a/src/util/util.c b/src/util/util.c index 3958a11..c7d5c05 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -316,7 +316,7 @@ static int virClearCapabilities(void) */ int virFork(pid_t *pid) { -# ifdef HAVE_PTHREAD_H +# ifdef HAVE_PTHREAD_SIGMASK sigset_t oldmask, newmask; # endif struct sigaction sig_action; @@ -328,7 +328,7 @@ int virFork(pid_t *pid) { * Need to block signals now, so that child process can safely * kill off caller's signal handlers without a race. */ -# ifdef HAVE_PTHREAD_H +# ifdef HAVE_PTHREAD_SIGMASK sigfillset(&newmask); if (pthread_sigmask(SIG_SETMASK, &newmask, &oldmask) != 0) { saved_errno = errno; @@ -349,7 +349,7 @@ int virFork(pid_t *pid) { virLogUnlock(); if (*pid < 0) { -# ifdef HAVE_PTHREAD_H +# ifdef HAVE_PTHREAD_SIGMASK /* attempt to restore signal mask, but ignore failure, to avoid obscuring the fork failure */ ignore_value (pthread_sigmask(SIG_SETMASK, &oldmask, NULL)); @@ -363,7 +363,7 @@ int virFork(pid_t *pid) { /* parent process */ -# ifdef HAVE_PTHREAD_H +# ifdef HAVE_PTHREAD_SIGMASK /* Restore our original signal mask now that the child is safely running */ if (pthread_sigmask(SIG_SETMASK, &oldmask, NULL) != 0) { @@ -407,7 +407,7 @@ int virFork(pid_t *pid) { sigaction(i, &sig_action, NULL); } -# ifdef HAVE_PTHREAD_H +# ifdef HAVE_PTHREAD_SIGMASK /* Unmask all signals in child, since we've no idea what the caller's done with their signal mask and don't want to propagate that to children */ -- 1.6.6.1
On 04/29/2010 05:41 AM, Eric Blake wrote:
Gnulib guarantees that pthread.h exists, but for now, it is a dummy header with no support for most pthread_* functions. Modify our use of pthread to use function checks, rather than header checks, to determine how much pthread support is present.
Shouldn't this be 1/5 for the sake of bisectability?
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 1917f26..b798cdf 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -9487,7 +9487,7 @@ remoteIOEventLoop(virConnectPtr conn, struct remote_thread_call *tmp = priv->waitDispatch; struct remote_thread_call *prev; char ignore; -#ifdef HAVE_PTHREAD_H +#ifdef HAVE_PTHREAD_SIGMASK sigset_t oldmask, blockedsigs; #endif
If there is no pthread_sigmask but there is sigprocmask, it is correct to use that too. But this should be done in gnulib though, so I suppose the patch is fine as is. Paolo
On 04/29/2010 01:14 AM, Paolo Bonzini wrote:
On 04/29/2010 05:41 AM, Eric Blake wrote:
Gnulib guarantees that pthread.h exists, but for now, it is a dummy header with no support for most pthread_* functions. Modify our use of pthread to use function checks, rather than header checks, to determine how much pthread support is present.
Shouldn't this be 1/5 for the sake of bisectability?
1/5 passes 'make check' independently on Linux, but I see your point at how it will fail on mingw (link errors) if this is not squashed in. So I'll go ahead and squash 1 and 2 into a single patch before pushing (I'm also trying to get a mingw build going before I push).
-#ifdef HAVE_PTHREAD_H +#ifdef HAVE_PTHREAD_SIGMASK sigset_t oldmask, blockedsigs; #endif
If there is no pthread_sigmask but there is sigprocmask, it is correct to use that too. But this should be done in gnulib though, so I suppose the patch is fine as is.
Ultimately, it would be nice if gnulib guaranteed pthread_sigmask, but we're not there yet. You are right, though, that gnulib guarantees sigprocmask, even form mingw, so it will probably be worth investigating in a future patch. At any rate, incremental steps are better than nothing :) -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
Eric Blake wrote:
Gnulib guarantees that pthread.h exists, but for now, it is a dummy header with no support for most pthread_* functions. Modify our use of pthread to use function checks, rather than header checks, to determine how much pthread support is present.
* configure.ac: Optimize function checks. Add check for pthread functions. * src/remote/remote_driver.c (remoteIOEventLoop): Depend on pthread_sigmask, now that gnulib guarantees pthread.h. * src/util/util.c (virFork): Likewise. * src/util/threads.c (threads-pthread.c): Depend on pthread_mutexattr_init, as a witness of full pthread support. * src/util/threads.h (threads-pthread.h): Likewise. --- configure.ac | 13 +++++++++++-- src/remote/remote_driver.c | 6 +++--- src/util/threads.c | 4 ++-- src/util/threads.h | 4 ++-- src/util/util.c | 10 +++++----- 5 files changed, 23 insertions(+), 14 deletions(-)
diff --git a/configure.ac b/configure.ac index cfefc02..b5ff348 100644 --- a/configure.ac +++ b/configure.ac @@ -107,10 +107,19 @@ dnl Use --disable-largefile if you don't want this. AC_SYS_LARGEFILE
dnl Availability of various common functions (non-fatal if missing). -AC_CHECK_FUNCS([cfmakeraw regexec uname sched_getaffinity getuid getgid posix_fallocate mmap]) +AC_CHECK_FUNCS_ONCE([cfmakeraw regexec uname sched_getaffinity getuid getgid \ + posix_fallocate mmap])
dnl Availability of various not common threadsafe functions -AC_CHECK_FUNCS([strerror_r strtok_r getmntent_r getgrnam_r getpwuid_r]) +AC_CHECK_FUNCS_ONCE([strerror_r strtok_r getmntent_r getgrnam_r getpwuid_r]) + +dnl Availability of pthread functions (if missing, win32 threading is +dnl assumed). Because of $LIB_PTHREAD, we cannot use AC_CHECK_FUNCS_ONCE. +dnl LIB_PTHREAD was set during gl_INIT by gnulib. +old_LIBS=$LIBS +LIBS="$LIBS $LIB_PTHREAD" +AC_CHECK_FUNCS([pthread_sigmask pthread_mutexattr_init]) +LIBS=$old_libs
dnl Availability of various common headers (non-fatal if missing). AC_CHECK_HEADERS([pwd.h paths.h regex.h sys/syslimits.h sys/utsname.h \ diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 1917f26..b798cdf 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -9487,7 +9487,7 @@ remoteIOEventLoop(virConnectPtr conn, struct remote_thread_call *tmp = priv->waitDispatch; struct remote_thread_call *prev; char ignore; -#ifdef HAVE_PTHREAD_H +#ifdef HAVE_PTHREAD_SIGMASK
Good change. This more in the spirit of the Autoconf Way. ACK.
* bootstrap.conf (gnulib_modules): Add uname. * configure.ac: Drop uname and sys/utsname.h checks. * src/nodeinfo.c (nodeGetInfo): Use uname unconditionally. --- bootstrap.conf | 1 + configure.ac | 4 ++-- src/nodeinfo.c | 12 ++---------- 3 files changed, 5 insertions(+), 12 deletions(-) diff --git a/bootstrap.conf b/bootstrap.conf index da7cc9c..e85f869 100644 --- a/bootstrap.conf +++ b/bootstrap.conf @@ -59,6 +59,7 @@ strtok_r sys_stat time_r timegm +uname useless-if-before-free usleep vasprintf diff --git a/configure.ac b/configure.ac index b5ff348..9a983a7 100644 --- a/configure.ac +++ b/configure.ac @@ -107,7 +107,7 @@ dnl Use --disable-largefile if you don't want this. AC_SYS_LARGEFILE dnl Availability of various common functions (non-fatal if missing). -AC_CHECK_FUNCS_ONCE([cfmakeraw regexec uname sched_getaffinity getuid getgid \ +AC_CHECK_FUNCS_ONCE([cfmakeraw regexec sched_getaffinity getuid getgid \ posix_fallocate mmap]) dnl Availability of various not common threadsafe functions @@ -122,7 +122,7 @@ AC_CHECK_FUNCS([pthread_sigmask pthread_mutexattr_init]) LIBS=$old_libs dnl Availability of various common headers (non-fatal if missing). -AC_CHECK_HEADERS([pwd.h paths.h regex.h sys/syslimits.h sys/utsname.h \ +AC_CHECK_HEADERS([pwd.h paths.h regex.h sys/syslimits.h \ sys/wait.h sched.h termios.h sys/poll.h syslog.h mntent.h net/ethernet.h]) dnl Where are the XDR functions? diff --git a/src/nodeinfo.c b/src/nodeinfo.c index 4d7fac1..5ec1bcf 100644 --- a/src/nodeinfo.c +++ b/src/nodeinfo.c @@ -29,16 +29,13 @@ #include <stdint.h> #include <errno.h> #include <dirent.h> +#include <sys/utsname.h> #if HAVE_NUMACTL # define NUMA_VERSION1_COMPATIBILITY 1 # include <numa.h> #endif -#ifdef HAVE_SYS_UTSNAME_H -# include <sys/utsname.h> -#endif - #include "c-ctype.h" #include "memory.h" #include "nodeinfo.h" @@ -273,18 +270,13 @@ int linuxNodeInfoCPUPopulate(FILE *cpuinfo, #endif int nodeGetInfo(virConnectPtr conn ATTRIBUTE_UNUSED, virNodeInfoPtr nodeinfo) { - memset(nodeinfo, 0, sizeof(*nodeinfo)); - -#ifdef HAVE_UNAME - { struct utsname info; + memset(nodeinfo, 0, sizeof(*nodeinfo)); uname(&info); if (virStrcpyStatic(nodeinfo->model, info.machine) == NULL) return -1; - } -#endif /* !HAVE_UNAME */ #ifdef __linux__ { -- 1.6.6.1
Eric Blake wrote:
* bootstrap.conf (gnulib_modules): Add uname. * configure.ac: Drop uname and sys/utsname.h checks. * src/nodeinfo.c (nodeGetInfo): Use uname unconditionally. --- bootstrap.conf | 1 + configure.ac | 4 ++-- src/nodeinfo.c | 12 ++----------
Straightforward. ACK
* configure.ac: Drop sys/wait.h check. * src/libvirt.c (includes): Use header unconditionally. * src/remote/remote_driver.c (includes): Likewise. * src/storage/storage_backend.c (includes): Likewise. * src/util/ebtables.c (includes): Likewise. * src/util/hooks.c (includes): Likewise. * src/util/iptables.c (includes): Likewise. * src/util/util.c (includes): Likewise. --- bootstrap.conf | 1 + configure.ac | 2 +- src/libvirt.c | 4 +--- src/remote/remote_driver.c | 5 +---- src/storage/storage_backend.c | 4 +--- src/util/ebtables.c | 7 ++----- src/util/hooks.c | 4 +--- src/util/iptables.c | 7 ++----- src/util/util.c | 4 +--- 9 files changed, 11 insertions(+), 27 deletions(-) diff --git a/bootstrap.conf b/bootstrap.conf index e85f869..baf0bc2 100644 --- a/bootstrap.conf +++ b/bootstrap.conf @@ -57,6 +57,7 @@ strptime strsep strtok_r sys_stat +sys_wait time_r timegm uname diff --git a/configure.ac b/configure.ac index 9a983a7..26e10e0 100644 --- a/configure.ac +++ b/configure.ac @@ -123,7 +123,7 @@ LIBS=$old_libs dnl Availability of various common headers (non-fatal if missing). AC_CHECK_HEADERS([pwd.h paths.h regex.h sys/syslimits.h \ - sys/wait.h sched.h termios.h sys/poll.h syslog.h mntent.h net/ethernet.h]) + sched.h termios.h sys/poll.h syslog.h mntent.h net/ethernet.h]) dnl Where are the XDR functions? dnl If portablexdr is installed, prefer that. diff --git a/src/libvirt.c b/src/libvirt.c index ff36681..d6f7f21 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -18,9 +18,7 @@ #include <sys/stat.h> #include <unistd.h> #include <assert.h> -#ifdef HAVE_SYS_WAIT_H -# include <sys/wait.h> -#endif +#include <sys/wait.h> #include <time.h> #include <gcrypt.h> diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index b798cdf..ef7a229 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -33,6 +33,7 @@ #include <sys/stat.h> #include <fcntl.h> #include <arpa/inet.h> +#include <sys/wait.h> /* Windows socket compatibility functions. */ #include <errno.h> @@ -45,10 +46,6 @@ # include <netinet/tcp.h> #endif -#ifdef HAVE_SYS_WAIT_H -# include <sys/wait.h> -#endif - #ifdef HAVE_PWD_H # include <pwd.h> #endif diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index f0074ed..4d34f9a 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -29,9 +29,7 @@ # include <regex.h> #endif #include <sys/types.h> -#if HAVE_SYS_WAIT_H -# include <sys/wait.h> -#endif +#include <sys/wait.h> #include <unistd.h> #include <fcntl.h> #include <stdint.h> diff --git a/src/util/ebtables.c b/src/util/ebtables.c index a6afdf8..e2b9608 100644 --- a/src/util/ebtables.c +++ b/src/util/ebtables.c @@ -1,6 +1,6 @@ /* - * Copyright (C) 2009 IBM Corp. * Copyright (C) 2007-2010 Red Hat, Inc. + * Copyright (C) 2009 IBM Corp. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -33,10 +33,7 @@ #include <fcntl.h> #include <sys/types.h> #include <sys/stat.h> - -#ifdef HAVE_SYS_WAIT_H -# include <sys/wait.h> -#endif +#include <sys/wait.h> #ifdef HAVE_PATHS_H # include <paths.h> diff --git a/src/util/hooks.c b/src/util/hooks.c index 507029f..dec9223 100644 --- a/src/util/hooks.c +++ b/src/util/hooks.c @@ -24,9 +24,7 @@ #include <config.h> #include <sys/types.h> -#if HAVE_SYS_WAIT_H -# include <sys/wait.h> -#endif +#include <sys/wait.h> #include <sys/stat.h> #include <unistd.h> #include <stdlib.h> diff --git a/src/util/iptables.c b/src/util/iptables.c index facc4da..4f95a02 100644 --- a/src/util/iptables.c +++ b/src/util/iptables.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2007-2009 Red Hat, Inc. + * Copyright (C) 2007-2010 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -31,10 +31,7 @@ #include <fcntl.h> #include <sys/types.h> #include <sys/stat.h> - -#ifdef HAVE_SYS_WAIT_H -# include <sys/wait.h> -#endif +#include <sys/wait.h> #ifdef HAVE_PATHS_H # include <paths.h> diff --git a/src/util/util.c b/src/util/util.c index c7d5c05..b2dd2ed 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -37,9 +37,7 @@ #include <sys/types.h> #include <sys/stat.h> #include <sys/ioctl.h> -#if HAVE_SYS_WAIT_H -# include <sys/wait.h> -#endif +#include <sys/wait.h> #if HAVE_MMAP # include <sys/mman.h> #endif -- 1.6.6.1
Eric Blake wrote:
* configure.ac: Drop sys/wait.h check. * src/libvirt.c (includes): Use header unconditionally. * src/remote/remote_driver.c (includes): Likewise. * src/storage/storage_backend.c (includes): Likewise. * src/util/ebtables.c (includes): Likewise. * src/util/hooks.c (includes): Likewise. * src/util/iptables.c (includes): Likewise. * src/util/util.c (includes): Likewise. --- bootstrap.conf | 1 + configure.ac | 2 +- src/libvirt.c | 4 +--- src/remote/remote_driver.c | 5 +---- src/storage/storage_backend.c | 4 +--- src/util/ebtables.c | 7 ++----- src/util/hooks.c | 4 +--- src/util/iptables.c | 7 ++----- src/util/util.c | 4 +--- 9 files changed, 11 insertions(+), 27 deletions(-)
diff --git a/bootstrap.conf b/bootstrap.conf index e85f869..baf0bc2 100644 --- a/bootstrap.conf +++ b/bootstrap.conf @@ -57,6 +57,7 @@ strptime strsep strtok_r sys_stat +sys_wait time_r timegm uname diff --git a/configure.ac b/configure.ac index 9a983a7..26e10e0 100644 --- a/configure.ac +++ b/configure.ac @@ -123,7 +123,7 @@ LIBS=$old_libs
dnl Availability of various common headers (non-fatal if missing). AC_CHECK_HEADERS([pwd.h paths.h regex.h sys/syslimits.h \ - sys/wait.h sched.h termios.h sys/poll.h syslog.h mntent.h net/ethernet.h]) + sched.h termios.h sys/poll.h syslog.h mntent.h net/ethernet.h])
Reviewing this made me realize I hadn't performed this clean-up in coreutils. There were three. FYI, coreutils' m4/stat-prog.m4 is the exception: I let it keep its check for netinet/in.h and the "#if HAVE_NETINET_IN_H" guard, even though gnulib provides it. Otherwise, this macro would have to depend on the netinet_in module. That would be fine if stat-prog.m4 were a gnulib module, but it's not...
diff --git a/src/libvirt.c b/src/libvirt.c index ff36681..d6f7f21 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -18,9 +18,7 @@ #include <sys/stat.h> #include <unistd.h> #include <assert.h> -#ifdef HAVE_SYS_WAIT_H -# include <sys/wait.h> -#endif +#include <sys/wait.h>
Mostly mechanical. ACK.
* configure.ac (AC_CHECK_FUNCS_ONCE, AC_SYS_LARGEFILE): Rely on gnulib for strtok_r and large file support. (AC_OBJEXT): Drop call now done by AC_PROG_CC. (m4_foreach_w): Drop macro guaranteed by gnulib. (AC_C_CONST): Drop call declared obsolete by autoconf. Signed-off-by: Eric Blake <eblake@redhat.com> --- configure.ac | 16 +--------------- 1 files changed, 1 insertions(+), 15 deletions(-) diff --git a/configure.ac b/configure.ac index 26e10e0..795529c 100644 --- a/configure.ac +++ b/configure.ac @@ -48,20 +48,10 @@ AC_PROG_CC AC_PROG_INSTALL AC_PROG_CPP -AC_OBJEXT - -dnl gl_INIT uses m4_foreach_w, yet that is not defined in autoconf-2.59. -dnl In order to accommodate developers with such old tools, here's a -dnl replacement definition. -m4_ifndef([m4_foreach_w], - [m4_define([m4_foreach_w], - [m4_foreach([$1], m4_split(m4_normalize([$2]), [ ]), [$3])])]) - gl_EARLY gl_INIT AM_PROG_CC_STDC -AC_C_CONST AC_TYPE_UID_T dnl Make sure we have an ANSI compiler @@ -102,16 +92,12 @@ fi AC_MSG_RESULT([$have_cpuid]) -dnl Support large files / 64 bit seek offsets. -dnl Use --disable-largefile if you don't want this. -AC_SYS_LARGEFILE - dnl Availability of various common functions (non-fatal if missing). AC_CHECK_FUNCS_ONCE([cfmakeraw regexec sched_getaffinity getuid getgid \ posix_fallocate mmap]) dnl Availability of various not common threadsafe functions -AC_CHECK_FUNCS_ONCE([strerror_r strtok_r getmntent_r getgrnam_r getpwuid_r]) +AC_CHECK_FUNCS_ONCE([strerror_r getmntent_r getgrnam_r getpwuid_r]) dnl Availability of pthread functions (if missing, win32 threading is dnl assumed). Because of $LIB_PTHREAD, we cannot use AC_CHECK_FUNCS_ONCE. -- 1.6.6.1
Eric Blake wrote:
* configure.ac (AC_CHECK_FUNCS_ONCE, AC_SYS_LARGEFILE): Rely on gnulib for strtok_r and large file support. (AC_OBJEXT): Drop call now done by AC_PROG_CC. (m4_foreach_w): Drop macro guaranteed by gnulib. (AC_C_CONST): Drop call declared obsolete by autoconf.
Signed-off-by: Eric Blake <eblake@redhat.com> --- configure.ac | 16 +--------------- 1 files changed, 1 insertions(+), 15 deletions(-)
diff --git a/configure.ac b/configure.ac index 26e10e0..795529c 100644 --- a/configure.ac +++ b/configure.ac @@ -48,20 +48,10 @@ AC_PROG_CC AC_PROG_INSTALL AC_PROG_CPP
-AC_OBJEXT - -dnl gl_INIT uses m4_foreach_w, yet that is not defined in autoconf-2.59. -dnl In order to accommodate developers with such old tools, here's a -dnl replacement definition. -m4_ifndef([m4_foreach_w], - [m4_define([m4_foreach_w], - [m4_foreach([$1], m4_split(m4_normalize([$2]), [ ]), [$3])])]) - gl_EARLY gl_INIT
AM_PROG_CC_STDC -AC_C_CONST AC_TYPE_UID_T
dnl Make sure we have an ANSI compiler @@ -102,16 +92,12 @@ fi AC_MSG_RESULT([$have_cpuid])
-dnl Support large files / 64 bit seek offsets. -dnl Use --disable-largefile if you don't want this. -AC_SYS_LARGEFILE - dnl Availability of various common functions (non-fatal if missing). AC_CHECK_FUNCS_ONCE([cfmakeraw regexec sched_getaffinity getuid getgid \ posix_fallocate mmap])
dnl Availability of various not common threadsafe functions -AC_CHECK_FUNCS_ONCE([strerror_r strtok_r getmntent_r getgrnam_r getpwuid_r]) +AC_CHECK_FUNCS_ONCE([strerror_r getmntent_r getgrnam_r getpwuid_r])
Good clean-up ideas. coreutils also has/had a use of AC_SYS_LARGEFILE. There must be a good syntax check addition in here... ACK.
On Wed, Apr 28, 2010 at 09:41:44PM -0600, Eric Blake wrote:
I can't yet upgrade to the latest gnulib due to some LGPLv2+ issues being worked out upstream. But in the meantime, here's some cleanups that will make it easier to upgrade when gnulib is ready. I always like patch series with a net reduction in lines.
I suppose I should try to get a mingw or cygwin build finished with these in place, before pushing anything, but that will have to be tomorrow (I tested on Linux, but it's late for me now).
Hum, I agree the reduction looks a good idea. But since this isn't really tied to a bug I would prefer to postpone that patchset to after 0.8.1, i.e. for next week, okay ? 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 04/29/2010 08:03 AM, Daniel Veillard wrote:
On Wed, Apr 28, 2010 at 09:41:44PM -0600, Eric Blake wrote:
I can't yet upgrade to the latest gnulib due to some LGPLv2+ issues being worked out upstream. But in the meantime, here's some cleanups that will make it easier to upgrade when gnulib is ready. I always like patch series with a net reduction in lines.
I suppose I should try to get a mingw or cygwin build finished with these in place, before pushing anything, but that will have to be tomorrow (I tested on Linux, but it's late for me now).
Hum, I agree the reduction looks a good idea. But since this isn't really tied to a bug I would prefer to postpone that patchset to after 0.8.1, i.e. for next week,
okay ?
Concur; patchset has been tabled to post-release. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
participants (4)
-
Daniel Veillard -
Eric Blake -
Jim Meyering -
Paolo Bonzini