[libvirt PATCH] Do not disable incompatible-pointer-types-discards-qualifiers

Martin Kletzander mkletzan at redhat.com
Mon Nov 16 09:52:54 UTC 2020


On Mon, Nov 16, 2020 at 09:24:22AM +0000, Daniel P. Berrangé wrote:
>On Sun, Nov 15, 2020 at 09:42:40PM +0100, Martin Kletzander wrote:
>> This reverts commit b3710e9a2af402a2b620de570b062294e11190eb.
>>
>> That check is very valuable for our code, but it causes issue with glib >=
>> 2.67.0 when building with clang.
>>
>> The reason is a combination of two commits in glib, firstly fdda405b6b1b which
>> adds a g_atomic_pointer_{set,get} variants that enforce stricter type
>> checking (by removing an extra cast) for compilers that support __typeof__, and
>> commit dce24dc4492d which effectively enabled the new variant of glib's atomic
>> code for clang.  This will not be necessary when glib's issue #600 [0] (8 years
>> old) is fixed.  Thankfully, MR #1719 [1], which is supposed to deal with this
>> issue was opened 3 weeks ago, so there is a slight sliver of hope.
>>
>> [0] https://gitlab.gnome.org/GNOME/glib/-/issues/600
>> [1] https://gitlab.gnome.org/GNOME/glib/-/merge_requests/1719
>>
>> Signed-off-by: Martin Kletzander <mkletzan at redhat.com>
>> ---
>>  meson.build               | 3 ---
>>  src/qemu/qemu_domain.c    | 7 +++++++
>>  src/util/vireventthread.c | 6 ++++++
>>  src/util/viridentity.c    | 6 ++++++
>>  src/util/virobject.c      | 6 ++++++
>>  5 files changed, 25 insertions(+), 3 deletions(-)
>>
>> diff --git a/meson.build b/meson.build
>> index cecaad199d4c..04646e3a078c 100644
>> --- a/meson.build
>> +++ b/meson.build
>> @@ -405,9 +405,6 @@ cc_flags += [
>>    # so use this Clang-specific arg to keep it quiet
>>    '-Wno-typedef-redefinition',
>>
>> -  # Clang complains about casts in G_DEFINE_TYPE(...)
>> -  '-Wno-incompatible-pointer-types-discards-qualifiers',
>> -
>>    # We don't use -Wc++-compat so we have to enable it explicitly
>>    '-Wjump-misses-init',
>>
>> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
>> index 2158080a56ad..3349476cceae 100644
>> --- a/src/qemu/qemu_domain.c
>> +++ b/src/qemu/qemu_domain.c
>> @@ -330,7 +330,14 @@ struct _qemuDomainLogContext {
>>      virLogManagerPtr manager;
>>  };
>>
>> +#pragma clang diagnostic push
>> +/* Workaround for glib's issue #600 on clang */
>> +#pragma clang diagnostic ignored "-Wincompatible-pointer-types-discards-qualifiers"
>> +
>>  G_DEFINE_TYPE(qemuDomainLogContext, qemu_domain_log_context, G_TYPE_OBJECT);
>> +
>> +#pragma clang diagnostic pop
>
>I really don't want to see this pattern used. We may only have a handful
>of G_DEFINE_TYPE today, but as we convert all 200  virClass over to GObject
>this is going to be way to tedious.  This has to be done as a global
>thing IMHO.
>

Even when the warning we're disabling actually catches errors we make every now
and then?  Really?

I know the issue is very old and it only showed up now.  And that the MR might
not go through for quite some time.  But how about a middle ground like the one
I described in reply to Pavel?  Looks like this:

#define G_DEFINE_TYPE_WITH_WORKAROUND(a, b, c) \
     _Pragma ("clang diagnostic push") \
     _Pragma ("clang diagnostic ignored \"-Wincompatible-pointer-types-discards-qualifiers\"") \
     G_DEFINE_TYPE(a, b, c) \
     _Pragma ("clang diagnostic pop")

If someone wants to make sure it does not break and we really want to get rid of
this ASAP, then we can even syntax-check for it, describe it and only use it
with glib that has this issue.

>
>Regards,
>Daniel
>-- 
>|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
>|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
>|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20201116/f8bc7233/attachment-0001.sig>


More information about the libvir-list mailing list