[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC 00/15] Error API: Flag errors in *errp even if err
From: |
Daniel P. Berrange |
Subject: |
Re: [Qemu-devel] [RFC 00/15] Error API: Flag errors in *errp even if errors are being ignored |
Date: |
Thu, 29 Jun 2017 15:01:42 +0100 |
User-agent: |
Mutt/1.8.0 (2017-02-23) |
On Wed, Jun 28, 2017 at 02:41:58PM -0300, Eduardo Habkost wrote:
> On Wed, Jun 28, 2017 at 11:05:26AM +0200, Markus Armbruster wrote:
> > > Ensuring errp is never NULL
> > > ---------------------------
> > >
> > > The last patch on this series changes the (Error **errp)
> > > parameters in functions to (Error *errp[static 1]), just to help
> > > validate the existing code, as clang warns about NULL arguments
> > > on that case. I don't think we should apply that patch, though,
> > > because the "[static 1]" syntax confuses Coccinelle.
> > >
> > > I have a branch where I experimented with the idea of replacing
> > > (Error **errp) parameters with an opaque type (void*, or a struct
> > > type). I gave up when I noticed it would require touching all
> > > callers to replace &err with a wrapper macro to convert to the
> > > right type. Suggestions to make NULL errp easier to detect at
> > > build time are welcome.
> > >
> > > (Probably the easiest solution for that is to add assert(errp)
> > > lines to the ERR_IS_*() macros.)
> >
> > We'll obviously struggle with null arguments until all the developers
> > adjusted to the new interface. Possibly with occasional mistakes
> > forever. Compile-time checking would really, really help.
>
> True. I'm investigating the possibility of using
> __attribute__((nonull(...))) with Coccinelle's help.
Beware that '__attribute__((nonnull))' has two distinct effects,
one of which is a potentially nasty trap which leads to crashes....
The useful part is that it allows compilers & analysis tools
like coverity to warn if you accidentally pass NULL into
a method. These warnings, particularly from gcc, only catch
a fraction of scenarios where you pass NULL in though.
The less useful part is that if GCC sees a nonnull annotation
on a parameter, then in the body of the method, it will silently
remove any code which does "if (!paramname)". So if you added
a check for the parameter being NULL to avoid a crash, gcc will
remove that protection, so you'll once again get a crash at
runtime if passing NULL.
So if you use the nonnull annotation, they you probably want
to make sure to pass -fno-delete-null-pointer-checks to
GCC to stop it removing your protection code, or you need to
be very confident that nothing will mistakenly pass NULL into
the methods annotated nonnull.
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 :|
- Re: [Qemu-devel] [RFC 15/15] [test only] Use 'Error *err[static 1]' instead of 'Error **errp' to catch NULL errp arguments, (continued)
Re: [Qemu-devel] [RFC 00/15] Error API: Flag errors in *errp even if errors are being ignored, Eric Blake, 2017/06/27
Re: [Qemu-devel] [RFC 00/15] Error API: Flag errors in *errp even if errors are being ignored, Markus Armbruster, 2017/06/28
Re: [Qemu-devel] [RFC 00/15] Error API: Flag errors in *errp even if errors are being ignored,
Daniel P. Berrange <=
Re: [Qemu-devel] [RFC 00/15] Error API: Flag errors in *errp even if errors are being ignored, Paolo Bonzini, 2017/06/29
- Re: [Qemu-devel] [RFC 00/15] Error API: Flag errors in *errp even if errors are being ignored, Daniel P. Berrange, 2017/06/29
- Re: [Qemu-devel] [RFC 00/15] Error API: Flag errors in *errp even if errors are being ignored, Eduardo Habkost, 2017/06/29
- Re: [Qemu-devel] [RFC 00/15] Error API: Flag errors in *errp even if errors are being ignored, Daniel P. Berrange, 2017/06/29
- Re: [Qemu-devel] [RFC 00/15] Error API: Flag errors in *errp even if errors are being ignored, Eduardo Habkost, 2017/06/29
- Re: [Qemu-devel] [RFC 00/15] Error API: Flag errors in *errp even if errors are being ignored, Daniel P. Berrange, 2017/06/29
Re: [Qemu-devel] [RFC 00/15] Error API: Flag errors in *errp even if errors are being ignored, Daniel P. Berrange, 2017/06/29