[libvirt] [PATCH] build: use more gnulib modules for simpler code

* .gnulib: Update to latest, for sigpipe and sigaction modules. * bootstrap.conf (gnulib_modules): Add siaction, sigpipe, strerror_r. * tools/virsh.c (vshSetupSignals) [!SIGPIPE]: Delete, now that gnulib guarantees it. (SA_SIGINFO): Define for mingw fallback. * src/util/virterror.c (virStrerror): Simplify, now that gnulib guarantees the POSIX interface. * configure.ac (AC_CHECK_FUNCS_ONCE): Drop redundant check. (AM_PROG_CC_STDC): Move earlier, to keep autoconf happy. --- Gnulib makes it possible to prefer the POSIX strerror_r signature everywhere, even on Linux. And even though mingw lacks SA_SIGINFO (even with gnulib), the virsh signal handler still works for mingw with gnulib's implementation of SIGPIPE with minimal effort. * .gnulib 9779055...e7853fe (3):
sigaction: relax license from LGPLv3+ to LGPLv2+ update from texinfo autoupdate
.gnulib | 2 +- bootstrap.conf | 3 +++ configure.ac | 13 ++++++------- src/util/virterror.c | 18 +----------------- tools/virsh.c | 17 +++++++++-------- 5 files changed, 20 insertions(+), 33 deletions(-) diff --git a/.gnulib b/.gnulib index 9779055..e7853fe 160000 --- a/.gnulib +++ b/.gnulib @@ -1 +1 @@ -Subproject commit 9779055889c2715b593930e39ead552759b5ddc2 +Subproject commit e7853fee14b339b6d5327c3b5d5de59b13c4e39e diff --git a/bootstrap.conf b/bootstrap.conf index a55fca2..88832d1 100644 --- a/bootstrap.conf +++ b/bootstrap.conf @@ -58,12 +58,15 @@ random_r sched send setsockopt +sigaction +sigpipe snprintf socket stpcpy strchrnul strndup strerror +strerror_r-posix strptime strsep strtok_r diff --git a/configure.ac b/configure.ac index 337ce17..574e952 100644 --- a/configure.ac +++ b/configure.ac @@ -49,11 +49,11 @@ dnl Checks for C compiler. AC_PROG_CC AC_PROG_INSTALL AC_PROG_CPP +AM_PROG_CC_STDC gl_EARLY gl_INIT -AM_PROG_CC_STDC AC_TYPE_UID_T dnl Make sure we have an ANSI compiler @@ -93,12 +93,11 @@ fi AC_MSG_RESULT([$have_cpuid]) -dnl Availability of various common functions (non-fatal if missing). -AC_CHECK_FUNCS_ONCE([cfmakeraw regexec sched_getaffinity getuid getgid initgroups \ - posix_fallocate mmap]) - -dnl Availability of various not common threadsafe functions -AC_CHECK_FUNCS_ONCE([strerror_r getmntent_r getgrnam_r getpwuid_r]) +dnl Availability of various common functions (non-fatal if missing), +dnl and various less common threadsafe functions +AC_CHECK_FUNCS_ONCE([cfmakeraw regexec sched_getaffinity getuid getgid \ + initgroups posix_fallocate mmap \ + 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. diff --git a/src/util/virterror.c b/src/util/virterror.c index 7543603..3dd6256 100644 --- a/src/util/virterror.c +++ b/src/util/virterror.c @@ -1250,7 +1250,7 @@ void virReportErrorHelper(virConnectPtr conn, * @errBuf: the buffer to save the error to * @errBufLen: the buffer length * - * Generate an erro string for the given errno + * Generate an error string for the given errno * * Returns a pointer to the error string, possibly indicating that the * error is unknown @@ -1260,24 +1260,8 @@ const char *virStrerror(int theerrno, char *errBuf, size_t errBufLen) int save_errno = errno; const char *ret; -#ifdef HAVE_STRERROR_R -# ifdef __USE_GNU - /* Annoying linux specific API contract */ - ret = strerror_r(theerrno, errBuf, errBufLen); -# else strerror_r(theerrno, errBuf, errBufLen); ret = errBuf; -# endif -#else - /* Mingw lacks strerror_r and its strerror is definitely not - * threadsafe, so safest option is to just print the raw errno - * value - we can at least reliably & safely look it up in the - * header files for debug purposes - */ - int n = snprintf(errBuf, errBufLen, "errno=%d", theerrno); - ret = (0 < n && n < errBufLen - ? errBuf : _("internal error: buffer too small")); -#endif errno = save_errno; return ret; } diff --git a/tools/virsh.c b/tools/virsh.c index 5b26c78..aba7945 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -497,16 +497,21 @@ out: */ static int disconnected = 0; /* we may have been disconnected */ -#ifdef SIGPIPE +/* Gnulib doesn't guarantee SA_SIGINFO support. */ +#ifndef SA_SIGINFO +# define SA_SIGINFO 0 +#endif + /* * vshCatchDisconnect: * * We get here when a SIGPIPE is being raised, we can't do much in the * handler, just save the fact it was raised */ -static void vshCatchDisconnect(int sig, siginfo_t * siginfo, - void* context ATTRIBUTE_UNUSED) { - if ((sig == SIGPIPE) || (siginfo->si_signo == SIGPIPE)) +static void vshCatchDisconnect(int sig, siginfo_t *siginfo, + void *context ATTRIBUTE_UNUSED) { + if ((sig == SIGPIPE) || + (SA_SIGINFO && siginfo->si_signo == SIGPIPE)) disconnected++; } @@ -526,10 +531,6 @@ vshSetupSignals(void) { sigaction(SIGPIPE, &sig_action, NULL); } -#else -static void -vshSetupSignals(void) {} -#endif /* * vshReconnect: -- 1.7.3.4

2011/1/17 Eric Blake <eblake@redhat.com>:
* .gnulib: Update to latest, for sigpipe and sigaction modules. * bootstrap.conf (gnulib_modules): Add siaction, sigpipe, strerror_r. * tools/virsh.c (vshSetupSignals) [!SIGPIPE]: Delete, now that gnulib guarantees it. (SA_SIGINFO): Define for mingw fallback. * src/util/virterror.c (virStrerror): Simplify, now that gnulib guarantees the POSIX interface. * configure.ac (AC_CHECK_FUNCS_ONCE): Drop redundant check. (AM_PROG_CC_STDC): Move earlier, to keep autoconf happy. ---
Gnulib makes it possible to prefer the POSIX strerror_r signature everywhere, even on Linux. And even though mingw lacks SA_SIGINFO (even with gnulib), the virsh signal handler still works for mingw with gnulib's implementation of SIGPIPE with minimal effort.
ACK. Matthias

On 01/18/2011 02:06 PM, Matthias Bolte wrote:
2011/1/17 Eric Blake <eblake@redhat.com>:
* .gnulib: Update to latest, for sigpipe and sigaction modules. * bootstrap.conf (gnulib_modules): Add siaction, sigpipe, strerror_r. * tools/virsh.c (vshSetupSignals) [!SIGPIPE]: Delete, now that gnulib guarantees it. (SA_SIGINFO): Define for mingw fallback. * src/util/virterror.c (virStrerror): Simplify, now that gnulib guarantees the POSIX interface. * configure.ac (AC_CHECK_FUNCS_ONCE): Drop redundant check. (AM_PROG_CC_STDC): Move earlier, to keep autoconf happy.
ACK.
Thanks; pushed. Note - this commit has the (unfortunate) side-effect that incrementally crossing this commit when doing bisection or other branch traversal may cause some spurious build failures. There are (at least) two issues that I ran into during my testing where we have some chicken-and-egg dependency tracking issues in the makefiles such that the incremental build fails, but where a fresh checkout will succeed. So, if you run into either issue, hopefully these hints will help you avoid spending the time of a complete fresh checkout: 1. On Linux, strerror_r has two different signatures, depending on whether you are targetting POSIX or _GNU_SOURCE. Prior to this patch, we used only the _GNU_SOURCE signature; after this patch, the code expects the POSIX signature (in spite of using _GNU_SOURCE). But gnulib (intentionally) won't regenerate gnulib/lib/string.h merely because we added another module to bootstrap.conf, so the build may fail when the current state of the generated header doesn't match the state of the code calling strerror_r. If this happens, 'rm -f gnulib/lib/string.h && make' is safe and will resolve the problem. 2. This patch introduces an ordering constraint that causes newer autoconf some grief about the location of AM_PROG_CC_STDC. This is fixed if you have the latest gnulib .m4 files in place. However, the makefile rules for determining when to rerun ./autogen.sh have a dependency glitch where sometimes they try to rerun autoconf prior to rerunning autogen.sh, therefore, the autoconf run will fail because the updated gnulib macros haven't yet been picked up. If this happens, 'git submodule update && ./bootstrap && make' will clear up the problem. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
participants (2)
-
Eric Blake
-
Matthias Bolte