Re: [PATCH v2] util/qemu-option: Document the get_opt_value() function

["Daniel P. Berrangé" <berrange@redhat.com>]

On Tue, Jul 07, 2020 at 04:14:40AM +0200, Philippe Mathieu-Daudé wrote:
> On 7/7/20 3:14 AM, Richard Henderson wrote:
> > On 6/29/20 12:08 AM, Philippe Mathieu-Daudé wrote:
> >> Coverity noticed commit 950c4e6c94 introduced a dereference before
> >> null check in get_opt_value (CID1391003):
> >>
> >>   In get_opt_value: All paths that lead to this null pointer
> >>   comparison already dereference the pointer earlier (CWE-476)
> >>
> >> We fixed this in commit 6e3ad3f0e31, but relaxed the check in commit
> >> 0c2f6e7ee99 because "No callers of get_opt_value() pass in a NULL
> >> for the 'value' parameter".
> >>
> >> Since this function is publicly exposed, it risks new users to do
> >> the same error again. Avoid that documenting the 'value' argument
> >> must not be NULL.
> > 
> > I think we should also add some use of __attribute__((nonnull(...))) to enforce
> > this within the compiler.
> > 
> > I recently did this without a qemu/compiler.h QEMU_FOO wrapper within
> > target/arm.  But the nonnull option has optional arguments, so it might be
> > difficult to wrap in macros.
> 
> I have this patch after your suggestion from last year:
> 
> +#if __has_attribute(nonnull)
> +# define QEMU_NONNULL(LIST) __attribute__((nonnull((LIST))))
> +#else
> +# define QEMU_NONNULL(LIST)
> +#endif

The if/else branch is not required, as both clang and gcc support
this, and they are our only supported compilers.


Beware that __attribute__((nonnul)) has side-effects, as it was
originally implemented as a hint for the optimizer. It allows it
to eliminate any code in the method that does a comparison to
NULL.  Historically it only generated warning messages in very
few scenarios involving a literal NULL. Only more recently with
-fanalyzer can it generate warnings about indirect passing of
NULL via variables.

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