GNOME Bugzilla – Bug 734479
G_VALUE_HOLDS etc. cause -Wcast-qual warnings for a const GValue *
Last modified: 2017-12-13 23:56:49 UTC
If you have code like this const GValue *cv = ...; if (G_VALUE_HOLDS_BOXED (cv)) ... and compile with gcc -Wcast-qual, you get a warning, because _G_TYPE_CVH casts cv to type (GValue *). For _G_TYPE_CVH, g_type_check_value_holds and, for that matter, g_type_check_value, a const GValue * is clearly sufficient: they only need to look at its contents. If someone objects "don't use -Wcast-qual, then", I do have some sympathy for that point of view; but it's easy to fix. (This is Debian bug <https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=604738> originally reported by Petter Reinholdtsen.)
Created attachment 282899 [details] [review] g_type_check_value, g_type_check_value_holds: accept const argument Conceptually, these functions clearly ought to be fine for a const structure. This avoids _G_TYPE_CVH (the implementation of G_TYPE_CHECK_VALUE_TYPE, G_VALUE_HOLDS, G_VALUE_HOLDS_BOXED etc.) needing to cast to a mutable GValue, which causes G_VALUE_HOLDS (cv, type) to issue warnings under gcc -Wcast-qual if cv is a const GValue *.
Review of attachment 282899 [details] [review]: Looks OK to me. This is technically a minor API break, but I can’t see how it can cause any problems. Existing code which: • Passes in a non-const GValue* will have the value cast to const without problems. • Passes in a const GValue* will now match the type without problems. • Passes in a const GValue* cast to non-const will have the value cast again to const without problems. It looks to me like this patch was never actually put into the Debian glib2.0 package, which is a bit odd. It would have been nice to get some testing that way. https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=604738
Pushed, thanks. There is enough time left in the 2.55.x cycle for any problems with the API break to get noticed. Attachment 282899 [details] pushed as 0c0b1bd - g_type_check_value, g_type_check_value_holds: accept const argument
(In reply to Philip Withnall from comment #2) > This is technically a minor API break, but I can’t see how > it can cause any problems. Anything that could validly be passed to the old functions and macros can be validly passed to the new functions and macros, I think? (It's OK to assign/pass/cast a Thing * to a const Thing *, just not the other way round). > It looks to me like this patch was never actually put into the Debian > glib2.0 package, which is a bit odd. I didn't want to make Debian-specific API changes without a very good reason, because if we do that, there's a danger of developers who run a Debian derivative and develop against its GLib producing code that only compiles on Debian and its derivatives.
(In reply to Simon McVittie from comment #4) > (In reply to Philip Withnall from comment #2) > > This is technically a minor API break, but I can’t see how > > it can cause any problems. > > Anything that could validly be passed to the old functions and macros can be > validly passed to the new functions and macros, I think? (It's OK to > assign/pass/cast a Thing * to a const Thing *, just not the other way round). Yeah, I’m fairly wary with constness changes. They never seem to have gone well in the past. > > It looks to me like this patch was never actually put into the Debian > > glib2.0 package, which is a bit odd. > > I didn't want to make Debian-specific API changes without a very good > reason, because if we do that, there's a danger of developers who run a > Debian derivative and develop against its GLib producing code that only > compiles on Debian and its derivatives. I suspected that might have been the case. Thanks for clarifying. I think this means that all the outstanding Debian bugs tagged against glib2.0 are now fixed in GLib master: https://bugs.debian.org/cgi-bin/pkgreport.cgi?include=tags%3Apatch&exclude=tags%3Apending&pend-exc=done&repeatmerged=no&src=glib2.0, fwiw.