On 01/17/2018 11:56 AM, Eric Blake wrote: > 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).
I have been there... >>> >>> 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. I wouldn't have thought of that, thanks! Phil.
signature.asc
Description: OpenPGP digital signature