On Wed, Jan 17, 2018 at 11:33:34AM -0300, Philippe Mathieu-Daudé wrote: > On 01/17/2018 10:32 AM, Daniel P. Berrange wrote: > > On Wed, Jan 17, 2018 at 10:18:19AM -0300, Philippe Mathieu-Daudé wrote: > >> Signed-off-by: Philippe Mathieu-Daudé <f4...@amsat.org> > >> --- > >> include/qemu/compiler.h | 2 ++ > >> 1 file changed, 2 insertions(+) > >> > >> diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h > >> index 340e5fdc09..d9b2489391 100644 > >> --- a/include/qemu/compiler.h > >> +++ b/include/qemu/compiler.h > >> @@ -26,6 +26,8 @@ > >> > >> #define QEMU_WARN_UNUSED_RESULT __attribute__((warn_unused_result)) > >> > >> +#define QEMU_WARN_NONNULL_ARGS(args...) __attribute__((nonnull(args))) > > > > If we take this, it should come with a warning attached to it, because > > it has really nasty behaviour with GCC. Consider code like: > > > > void foo(void *bar) __attribute__((nonnull(1))); > > > > ... > > > > void foo(void *bar) { if (!bar) return; } > > > > 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 :-( > > TIL! > > > After having a number of crashes in libvirt caused by gcc > > optimizing out checks for NULL, we now only define nonnull > > when running under static analysis (coverity) and not when > > compiling normally. > > > > https://libvirt.org/git/?p=libvirt.git;a=blob;f=src/internal.h;h=5895030415968d72200599e8a59bbf01ffc2d5a3;hb=HEAD#l162 > > 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 > > > The 2 functions you've added nonnull attrs to look safe enough, > > but people might unwittingly use this elsewhere in QEMU in future > > not realizing the side-effect it has. 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 :|