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