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@xxxxxxxxxx> > > --- > > 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 :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
