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(a)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 :|