Skip to contentSkip to conversation

Use glib's checks for use of too-new functions

PATCH In review Dr. David Alan Gilbert (git) 4 participants 8 messages May 29, 2015 – Jun 1, 2015

lore
Share & tools
lore
#0 lore raw

cover Use glib's checks for use of too-new functions

From: "Dr. David Alan Gilbert" <[email protected]>

Glib 2.32 adds a checking mechanism to warn of use of functions that are available in glib versions newer than a particular version.

This series enables that check (via a -D added during configure); with it you get warnings like:

/home/dgilbert/git/qemu-world3/vl.c: In function 'main':
/home/dgilbert/git/qemu-world3/vl.c:4385:5: warning: 'g_source_get_ready_time' is deprecated: Not available before 2.36 [-Wdeprecated-declarations]

     printf("Foo! %lld\n", (long long) g_source_get_ready_time(NULL));

Notes:
a) The other patch in the set fixes a configure error that caused a test never to be run even if there was a new enough glib
b) Peter said he would prefer something #included rather than added as a -D but then we'd have to change a whole bunch of files to make sure they do that before glib.h is included, and it wouldn't protect you from new code that forgot to do that.
c) Glib's older than 2.32 don't have the mechanism and thus don't complain
d) The earliest version marker that glib lets you check against is 2.26, which is what I've set it to. Since we allow 2.22 but 2.22 doesn't have this new check, it doesn't cause any warnings.
e) Build and make check'd on fedora 22 (glib 2.44), rhel6 (glib 2.26), and rhel5 with glib 2.22 built on it.

Dave

Dr. David Alan Gilbert (2):
Fix glib_subprocess test
Compile time checks for newer glib

 configure                      | 7 ++++++-
 include/glib-compat.h          | 6 ++++++
 tests/test-qdev-global-props.c | 2 +-
 tests/vhost-user-test.c        | 2 ++
 4 files changed, 15 insertions(+), 2 deletions(-)
Signature · 1 line
2.4.1
#1 lore raw

PATCH 1/2 Fix glib_subprocess test

From: "Dr. David Alan Gilbert" <[email protected]>

A typo means that the tests dependent on glib with subprocess support are never run.

Fixes 9d41401b90fa10b335d2e739149d36437cfbf622
---
 configure | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
configure +1 -1
index 9852aef..24ee0a4 100755
@@ -4776,7 +4776,7 @@ if test "$bluez" = "yes" ; then
   echo "CONFIG_BLUEZ=y" >> $config_host_mak
   echo "BLUEZ_CFLAGS=$bluez_cflags" >> $config_host_mak
 fi
-if test "glib_subprocess" = "yes" ; then
+if test "$glib_subprocess" = "yes" ; then
   echo "CONFIG_HAS_GLIB_SUBPROCESS_TESTS=y" >> $config_host_mak
 fi
 echo "GLIB_CFLAGS=$glib_cflags" >> $config_host_mak
Signature · 1 line
2.4.1
#2 lore raw

PATCH 2/2 Compile time checks for newer glib

In reply to Dr. David Alan Gilbert (git)

From: "Dr. David Alan Gilbert" <[email protected]>

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.

While mainly we're interested in the check from GLIB_VERSION_MAX_ALLOWED which checks we're not using anything too recent, the glib macros require that if we set that then we must also define GLIB_VERSION_MIN_REQUIRED which checks we're not using anything deprecated too long ago.

---
 configure                      | 5 +++++
 include/glib-compat.h          | 6 ++++++
 tests/test-qdev-global-props.c | 2 +-
 tests/vhost-user-test.c        | 2 ++
 4 files changed, 14 insertions(+), 1 deletion(-)
configure +5 -0
index 24ee0a4..5c6230a 100755
@@ -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
include/glib-compat.h +6 -0
index 318e000..cbd660c 100644
@@ -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 */
tests/test-qdev-global-props.c +1 -1
index 0be9835..896e6bf 100644
@@ -22,7 +22,7 @@
  * THE SOFTWARE.
  */
 
-#include <glib.h>
+#include "glib-compat.h"
 #include <stdint.h>
 
 #include "hw/qdev.h"
tests/vhost-user-test.c +2 -0
index 75fedf0..7aeb927 100644
@@ -8,6 +8,8 @@
  *
  */
 
+#undef GLIB_VERSION_MIN_REQUIRED
+#undef GLIB_VERSION_MAX_ALLOWED
 #define QEMU_GLIB_COMPAT_H
 #include <glib.h>
 
Signature · 1 line
2.4.1
#3 lore raw

Re: [Qemu-devel] [PATCH 2/2] Compile time checks for newer glib

Quoted 10 lines · Dr. David Alan Gilbert (git)

From: "Dr. David Alan Gilbert" <[email protected]>

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

#4 lore raw

Re: [Qemu-devel] [PATCH 2/2] Compile time checks for newer glib

Quoted 8 lines · * Paolo Bonzini ([email protected]) wrote:
Quoted 5 lines · Dr. David Alan Gilbert (git)

From: "Dr. David Alan Gilbert" <[email protected]>

Newer glib has support for checking that applications aren't using newer glib calls than they should be.

Quoted 8 lines
Quoted 5 lines

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

Quoted 2 lines

Paolo

--
Dr. David Alan Gilbert / [email protected] / Manchester, UK

#5 lore raw

Re: [Qemu-devel] [PATCH 2/2] Compile time checks for newer glib

In reply to Dr. David Alan Gilbert (git)

include/glib-compat.h line 25

Quoted 13 lines · Dr. David Alan Gilbert (git)
include/glib-compat.h +6 -0
@@ -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

#6 lore raw

Re: [Qemu-devel] [PATCH 2/2] Compile time checks for newer glib

Quoted 8 lines · * Peter Maydell ([email protected]) wrote:
Quoted 6 lines · Dr. David Alan Gilbert (git)
include/glib-compat.h +0 -0
@@ -16,6 +16,12 @@
 #ifndef QEMU_GLIB_COMPAT_H
 #define QEMU_GLIB_COMPAT_H
 
Quoted 11 lines

include/glib-compat.h line 25

Quoted 7 lines
include/glib-compat.h +6 -0
+/*
+ * 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

Quoted 2 lines

-- PMM

--
Dr. David Alan Gilbert / [email protected] / Manchester, UK

#7 lore raw

Re: [Qemu-devel] [PATCH 2/2] Compile time checks for newer glib

Quoted 26 lines · Dr. David Alan Gilbert
Quoted 8 lines · * Peter Maydell ([email protected]) wrote:
Quoted 6 lines · Dr. David Alan Gilbert (git)
include/glib-compat.h +0 -0
@@ -16,6 +16,12 @@
 #ifndef QEMU_GLIB_COMPAT_H
 #define QEMU_GLIB_COMPAT_H
 
Quoted 11 lines

include/glib-compat.h line 25

Quoted 7 lines
include/glib-compat.h +6 -0
+/*
+ * 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

Quoted 7 lines

Dave

Quoted 2 lines

-- PMM

--
Dr. David Alan Gilbert / [email protected] / 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