qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [RFC PATCH 1/3] compiler: add QEMU_WARN_NONNULL_ARGS()


From: Eric Blake
Subject: Re: [Qemu-devel] [RFC PATCH 1/3] compiler: add QEMU_WARN_NONNULL_ARGS()
Date: Wed, 17 Jan 2018 08:56:23 -0600
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.2

On 01/17/2018 08:39 AM, Daniel P. Berrange wrote:

>>>
>>> GCC may or may not warn you about passing NULL for the 'bar'
>>> parameter, but it will none the less assume nothing passes
>>> NULL, and thus remove the 'if (!bar)' conditional during
>>> optimization. IOW, adding nonnull annotations can actually
>>> make your code less robust :-(

The gcc bug mentioned in the libvirt code says that newer gcc may be
smarter than when we added the libvirt workaround:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=17308

But I haven't played with it in a long time (the sour experience with
misoptimized code with no warning has made me wary of re-trying).


>>
>> Why do you use __attribute__(()) ? Isn't this enough:
> 
> No idea offhand - Eric wrote this so perhaps he had a reason for that
> else branch style.
> 
>>
>> #if defined __clang_analyzer__ || defined __COVERITY__
>> #define QEMU_STATIC_ANALYSIS 1
>> +#define QEMU_WARN_NONNULL_ARGS(args...) __attribute__((nonnull(args)))
>> +#else
>> +#define QEMU_WARN_NONNULL_ARGS(args...)
>> #endif

My reasoning for using the empty attribute was to at least ensure that
the gcc compilation agrees that the annotation is syntactically valid
(nothing worse than sticking an __attribute__ code in a place that gcc
doesn't like, but only for static checker builds, so you don't learn
about it right away).  Defining the macro to nothing, rather than an
empty attribute, makes it harder for the common-case compilation to
detect placement problems.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]