[libvirt] [PATCH] Add missing checks for whether the connection is open in dispatcher

[Eric Blake]

On 04/13/2011 08:58 AM, Daniel P. Berrange wrote:
> Many functions did not check for whether a connection was
> open. Replace the macro which obscures control flow, with
> explicit checks, and ensure all dispatcher code has checks.
> 
> * daemon/remote.c: Add connection checks
> ---
>  daemon/remote.c |  986 +++++++++++++++++++++++++++++++++++++++++++++++++++++--
>  1 files changed, 959 insertions(+), 27 deletions(-)
> 
> diff --git a/daemon/remote.c b/daemon/remote.c
> index 8a2a71f..2a56d91 100644
> --- a/daemon/remote.c
> +++ b/daemon/remote.c
> @@ -432,11 +432,6 @@ remoteDispatchOpen(struct qemud_server *server,
>      return rc;
>  }
>  
> -#define CHECK_CONN(client)                                              \
> -    if (!client->conn) {                                                \
> -        remoteDispatchFormatError(rerr, "%s", _("connection not open")); \
> -        return -1;                                                      \
> -    }

Makes sense to expand the return inline.

>  
>  static int
>  remoteDispatchClose(struct qemud_server *server ATTRIBUTE_UNUSED,
> @@ -464,6 +459,12 @@ remoteDispatchSupportsFeature(struct qemud_server *server ATTRIBUTE_UNUSED,
>                                remote_error *rerr,
>                                remote_supports_feature_args *args, remote_supports_feature_ret *ret)
>  {
> +
> +    if (!conn) {
> +        remoteDispatchFormatError(rerr, "%s", _("connection not open"));
> +        return -1;
> +    }
> +
>      ret->supported = virDrvSupportsFeature(conn, args->feature);

And this was indeed one of the missing points.  I quickly scanned the
rest of the patch, and didn't spot any obvious mistakes.  I'm trusting
that you caught the entire file, and didn't spend too much time myself
looking for any missed instances.

> @@ -573,7 +594,11 @@ remoteDispatchGetUri(struct qemud_server *server ATTRIBUTE_UNUSED,
>                       remote_get_uri_ret *ret)
>  {
>      char *uri;
> -    CHECK_CONN(client);
> +
> +    if (!conn) {
> +        remoteDispatchFormatError(rerr, "%s", _("connection not open"));
> +        return -1;
> +    }

Interestingly enough, the old code was checking whether client->conn was
non-null, even though the client parameter was marked ATTRIBUTE_UNUSED;
whereas the new code is checking whether the conn parameter is non-null.
 But looking in dispatch.c, I do see that we were indeed calling:
conn = client->conn;
(data->fn)(server, client, conn, ...)

so this has the same effect.  I'm guessing that part of the rpc rewrite
will involve nuking the unused parameters from the dispatch functions.

ACK.

-- 
Eric Blake   eblake at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 620 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20110413/4df1bd3e/attachment-0001.sig>