Message ID | 1432911708-22845-3-git-send-email-dgilbert@redhat.com |
---|---|
State | New |
Headers | show |
On 29/05/2015 17:01, Dr. David Alan Gilbert (git) wrote: > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com> > > Newer glib has support for checking that applications aren't > using newer glib calls than they should be. > > The support for the check only went into glib 2.32 and it only > has macros for version 2.26 upwards; although we only insist > on 2.22 at the moment, I set the glib checks to the earliest of 2.26, > it wont cause problems on anything <2.32 since the checks aren't > there. I think we need 2.32 minimum, because we use the 2.32 mutex/condvar functions if available. Paolo
* Paolo Bonzini (pbonzini@redhat.com) wrote: > > > On 29/05/2015 17:01, Dr. David Alan Gilbert (git) wrote: > > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com> > > > > Newer glib has support for checking that applications aren't > > using newer glib calls than they should be. > > > > The support for the check only went into glib 2.32 and it only > > has macros for version 2.26 upwards; although we only insist > > on 2.22 at the moment, I set the glib checks to the earliest of 2.26, > > it wont cause problems on anything <2.32 since the checks aren't > > there. > > I think we need 2.32 minimum, because we use the 2.32 mutex/condvar > functions if available. Are all those done via glib-compat.h ? Note that in the patch I turn off the version checks at the top of glib-compat.h on the assumption that anything trying to use it knows what it is doing. Dave > > Paolo -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
On 29 May 2015 at 16:01, Dr. David Alan Gilbert (git) <dgilbert@redhat.com> wrote: > --- a/include/glib-compat.h > +++ b/include/glib-compat.h > @@ -16,6 +16,12 @@ > #ifndef QEMU_GLIB_COMPAT_H > #define QEMU_GLIB_COMPAT_H > > +/* > + * The source file including this compat header knows it's using newer glib > + * functions than we generally allow, so don't warn about it. > + */ > +#undef GLIB_VERSION_MIN_REQUIRED > +#undef GLIB_VERSION_MAX_ALLOWED > #include <glib.h> ...but glib-compat.h is included by qemu-common.h and in general the point is that code anywhere in QEMU can assume the compat versions exist, so it feels like this will defeat the point. -- PMM
* Peter Maydell (peter.maydell@linaro.org) wrote: > On 29 May 2015 at 16:01, Dr. David Alan Gilbert (git) > <dgilbert@redhat.com> wrote: > > --- a/include/glib-compat.h > > +++ b/include/glib-compat.h > > @@ -16,6 +16,12 @@ > > #ifndef QEMU_GLIB_COMPAT_H > > #define QEMU_GLIB_COMPAT_H > > > > +/* > > + * The source file including this compat header knows it's using newer glib > > + * functions than we generally allow, so don't warn about it. > > + */ > > +#undef GLIB_VERSION_MIN_REQUIRED > > +#undef GLIB_VERSION_MAX_ALLOWED > > #include <glib.h> > > ...but glib-compat.h is included by qemu-common.h and in general > the point is that code anywhere in QEMU can assume the compat > versions exist, so it feels like this will defeat the point. Hmm, a lot of files seem to include glib.h directly. But OK, so then Paolo is suggesting setting the minimum to 2.32 which I can do, but then that wouldn't stop something that's in 2.22-2.32 but not in glib-compat.h Dave > > -- PMM -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
On 05/29/2015 12:01 PM, Dr. David Alan Gilbert wrote: > * Peter Maydell (peter.maydell@linaro.org) wrote: >> On 29 May 2015 at 16:01, Dr. David Alan Gilbert (git) >> <dgilbert@redhat.com> wrote: >>> --- a/include/glib-compat.h >>> +++ b/include/glib-compat.h >>> @@ -16,6 +16,12 @@ >>> #ifndef QEMU_GLIB_COMPAT_H >>> #define QEMU_GLIB_COMPAT_H >>> >>> +/* >>> + * The source file including this compat header knows it's using newer glib >>> + * functions than we generally allow, so don't warn about it. >>> + */ >>> +#undef GLIB_VERSION_MIN_REQUIRED >>> +#undef GLIB_VERSION_MAX_ALLOWED >>> #include <glib.h> >> >> ...but glib-compat.h is included by qemu-common.h and in general >> the point is that code anywhere in QEMU can assume the compat >> versions exist, so it feels like this will defeat the point. > > Hmm, a lot of files seem to include glib.h directly. But OK, > so then Paolo is suggesting setting the minimum to 2.32 > which I can do, but then that wouldn't stop something that's > in 2.22-2.32 but not in glib-compat.h > > Dave > >> >> -- PMM > -- > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK > When I bumped glib to 2.22, I played with using these macros and forcing all files to include glib-compat and prohibiting direct includes of glib. The problem is without a minimum version set to 2.3x, as you've stated, the deprecated warnings will trigger for functions we added explicit compat wrappers for, and I could think of no sane way to "forgive" these usages as a one-time exception. Perhaps -- /perhaps/ -- you could have a qemu-internal glib.h that sets the version warnings and leave glib-compat alone without those includes, but this seems like a setup that is inviting frustration for future devs. --js
diff --git a/configure b/configure index 24ee0a4..5c6230a 100755 --- a/configure +++ b/configure @@ -2781,6 +2781,11 @@ fi glib_req_ver=2.22 glib_modules=gthread-2.0 +# Force warnings over use of newer glib features; 2.26 is the earliest +# version macro that is defined; eventually we should match +# glib_req_ver above +QEMU_CFLAGS="-DGLIB_VERSION_MIN_REQUIRED=GLIB_VERSION_2_26 $QEMU_CFLAGS" +QEMU_CFLAGS="-DGLIB_VERSION_MAX_ALLOWED=GLIB_VERSION_2_26 $QEMU_CFLAGS" if test "$modules" = yes; then glib_modules="$glib_modules gmodule-2.0" fi diff --git a/include/glib-compat.h b/include/glib-compat.h index 318e000..cbd660c 100644 --- a/include/glib-compat.h +++ b/include/glib-compat.h @@ -16,6 +16,12 @@ #ifndef QEMU_GLIB_COMPAT_H #define QEMU_GLIB_COMPAT_H +/* + * The source file including this compat header knows it's using newer glib + * functions than we generally allow, so don't warn about it. + */ +#undef GLIB_VERSION_MIN_REQUIRED +#undef GLIB_VERSION_MAX_ALLOWED #include <glib.h> /* GLIB version compatibility flags */ diff --git a/tests/test-qdev-global-props.c b/tests/test-qdev-global-props.c index 0be9835..896e6bf 100644 --- a/tests/test-qdev-global-props.c +++ b/tests/test-qdev-global-props.c @@ -22,7 +22,7 @@ * THE SOFTWARE. */ -#include <glib.h> +#include "glib-compat.h" #include <stdint.h> #include "hw/qdev.h" diff --git a/tests/vhost-user-test.c b/tests/vhost-user-test.c index 75fedf0..7aeb927 100644 --- a/tests/vhost-user-test.c +++ b/tests/vhost-user-test.c @@ -8,6 +8,8 @@ * */ +#undef GLIB_VERSION_MIN_REQUIRED +#undef GLIB_VERSION_MAX_ALLOWED #define QEMU_GLIB_COMPAT_H #include <glib.h>