[libvirt] [PATCH v2 07/11] rpc: annotate various parameters as being required to be non-NULL

[Daniel P. Berrangé]

On Thu, Feb 01, 2018 at 06:57:49AM -0500, John Ferlan wrote:
> 
> 
> On 01/23/2018 08:23 AM, Daniel P. Berrange wrote:
> > The server name and client data callbacks need to be non-NULL or the
> > system will crash at various times. This is particularly bad when some
> > of the crashes only occur post-exec restart.
> > 
> > Signed-off-by: Daniel P. Berrange <berrange at redhat.com>
> > ---
> >  src/rpc/virnetserver.h       | 7 +++++--
> >  src/rpc/virnetserverclient.h | 7 +++++--
> >  2 files changed, 10 insertions(+), 4 deletions(-)
> > 
> 
> My Coverity build was not happy this morning...
> 
> > diff --git a/src/rpc/virnetserverclient.h b/src/rpc/virnetserverclient.h
> > index 3c48759abc..4a0d3cc25e 100644
> > --- a/src/rpc/virnetserverclient.h
> > +++ b/src/rpc/virnetserverclient.h
> > @@ -72,14 +72,17 @@ virNetServerClientPtr virNetServerClientNew(unsigned long long id,
> >                                              virNetServerClientPrivNew privNew,
> >                                              virNetServerClientPrivPreExecRestart privPreExecRestart,
> >                                              virFreeCallback privFree,
> > -                                            void *privOpaque);
> > +                                            void *privOpaque)
> > +    ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(7) ATTRIBUTE_NONNULL(9);
> 
> This one caused the Coverity build to fail because virNetServerClientNew
> checks "if (privNew)" before assigning client->privateData
> 
> >  
> >  virNetServerClientPtr virNetServerClientNewPostExecRestart(virNetServerPtr srv,
> >                                                             virJSONValuePtr object,
> >                                                             virNetServerClientPrivNewPostExecRestart privNew,
> >                                                             virNetServerClientPrivPreExecRestart privPreExecRestart,
> >                                                             virFreeCallback privFree,
> > -                                                           void *privOpaque);
> > +                                                           void *privOpaque)
> > +    ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3)
> > +    ATTRIBUTE_NONNULL(4) ATTRIBUTE_NONNULL(5);
> 
> Likewise here too for virNetServerClientNewPostExecRestart w/ privNew

Fun, I'm trying to understand again how I caused the crash when these
were NULL. I think it was because in the virNetServerClientNew() call
they were non-NULL, but the virNetServerClientNewPostExecRestart()
I had passed NULL, or vica-verca.

Anyway since all production code callers pass non-NULL, I think these
should be marked NONNULL regardless of crash possibility....

> Finally, the "tests/virnetserverclienttest.c fails to build because of
> the NULL argument check.

...so I'll fix this test and remove the if (...) checks :-)


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