Pino Toscano
2015-Nov-09 10:41 UTC
[Libguestfs] [PATCH 1/2] lib: enable the libvirt code consistently everywhere
With commit bc2b41778405cc6a376a670703ce63e3678bf1fb HAVE_LIBVIRT_BACKEND is defined based on the libvirt version (using its version macro), although libvirt.h is included only after that check: because of this, variables in the guestfs_h struct after the HAVE_LIBVIRT_BACKEND block would be used wrongly if libvirt.h was not included before guestfs-internal.h, like in the recently added available.c (all the other places using libvirt features in the handle already happened to do so). Considering guestfs-internal.h already includes libvirt.h, move its inclusion up, right before the libvirt version check. --- src/guestfs-internal.h | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/guestfs-internal.h b/src/guestfs-internal.h index bc03ccc..a9f2f0d 100644 --- a/src/guestfs-internal.h +++ b/src/guestfs-internal.h @@ -38,12 +38,11 @@ MIN_LIBVIRT_MINOR * 1000 + \ MIN_LIBVIRT_MICRO) -#if defined(HAVE_LIBVIRT) && LIBVIR_VERSION_NUMBER >= MIN_LIBVIRT_VERSION -#define HAVE_LIBVIRT_BACKEND -#endif - #ifdef HAVE_LIBVIRT #include <libvirt/libvirt.h> +#if LIBVIR_VERSION_NUMBER >= MIN_LIBVIRT_VERSION +#define HAVE_LIBVIRT_BACKEND +#endif #endif #include "hash.h" -- 2.1.0
Pino Toscano
2015-Nov-09 10:41 UTC
[Libguestfs] [PATCH 2/2] lib: do not manually include libvirt.h in .c files
guestfs-internal.h includes it already, so rely on that. --- src/handle.c | 4 ---- src/launch-libvirt.c | 1 - src/libvirt-auth.c | 1 - src/libvirt-domain.c | 1 - 4 files changed, 7 deletions(-) diff --git a/src/handle.c b/src/handle.c index f7b0909..15fd104 100644 --- a/src/handle.c +++ b/src/handle.c @@ -23,10 +23,6 @@ #include <string.h> #include <libintl.h> -#ifdef HAVE_LIBVIRT -#include <libvirt/libvirt.h> -#endif - #include <libxml/parser.h> #include <libxml/xmlversion.h> diff --git a/src/launch-libvirt.c b/src/launch-libvirt.c index fae8e5d..9e108aa 100644 --- a/src/launch-libvirt.c +++ b/src/launch-libvirt.c @@ -33,7 +33,6 @@ #include <sys/un.h> /* sockaddr_un */ #ifdef HAVE_LIBVIRT -#include <libvirt/libvirt.h> #include <libvirt/virterror.h> #endif diff --git a/src/libvirt-auth.c b/src/libvirt-auth.c index dad61cd..cbc2461 100644 --- a/src/libvirt-auth.c +++ b/src/libvirt-auth.c @@ -25,7 +25,6 @@ #include <libintl.h> #ifdef HAVE_LIBVIRT -#include <libvirt/libvirt.h> #include <libvirt/virterror.h> #endif diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 124eef0..b096fb3 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -26,7 +26,6 @@ #include <libintl.h> #ifdef HAVE_LIBVIRT -#include <libvirt/libvirt.h> #include <libvirt/virterror.h> #endif -- 2.1.0
Richard W.M. Jones
2015-Nov-09 14:37 UTC
Re: [Libguestfs] [PATCH 1/2] lib: enable the libvirt code consistently everywhere
On Mon, Nov 09, 2015 at 11:41:30AM +0100, Pino Toscano wrote:> With commit bc2b41778405cc6a376a670703ce63e3678bf1fb > HAVE_LIBVIRT_BACKEND is defined based on the libvirt version (using its > version macro), although libvirt.h is included only after that check: > because of this, variables in the guestfs_h struct after the > HAVE_LIBVIRT_BACKEND block would be used wrongly if libvirt.h was not > included before guestfs-internal.h, like in the recently added > available.c (all the other places using libvirt features in the handle > already happened to do so).!!! Yes, I knew it would turn out to be memory corruption. ACK series. /me wonders how to avoid such problems in general ... 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