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:
#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
>
