Re: [libvirt] [PATCH] Add missing checks for whether the connection is open in dispatcher
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(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
Attachments:
- signature.asc (application/pgp-signature — 620 bytes)