On 12/11/18 4:56 PM, Daniel P. Berrangé wrote:
The make_nonnull_XXX methods can all fail due to OOM but this was
being
silently ignored and thus also not checked by callers. Make the methods
propagate errors and use ATTRIBUTE_RETURN_CHECK to force callers to deal
with it.
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
src/admin/admin_server_dispatch.c | 9 +-
src/remote/remote_daemon_dispatch.c | 393 +++++++++++++++++++++-------
src/rpc/gendispatch.pl | 19 +-
3 files changed, 315 insertions(+), 106 deletions(-)
diff --git a/src/admin/admin_server_dispatch.c b/src/admin/admin_server_dispatch.c
index b78ff902c0..9fa2893fa3 100644
--- a/src/admin/admin_server_dispatch.c
+++ b/src/admin/admin_server_dispatch.c
@@ -115,11 +115,13 @@ get_nonnull_server(virNetDaemonPtr dmn, admin_nonnull_server srv)
return virNetDaemonGetServer(dmn, srv.name);
}
-static void
+static int ATTRIBUTE_RETURN_CHECK
make_nonnull_server(admin_nonnull_server *srv_dst,
virNetServerPtr srv_src)
{
- ignore_value(VIR_STRDUP_QUIET(srv_dst->name, virNetServerGetName(srv_src)));
+ if (VIR_STRDUP(srv_dst->name, virNetServerGetName(srv_src)) < 0)
+ return -1;
+ return 0;
}
static virNetServerClientPtr
@@ -128,13 +130,14 @@ get_nonnull_client(virNetServerPtr srv, admin_nonnull_client clnt)
return virNetServerGetClient(srv, clnt.id);
}
-static void
+static int
make_nonnull_client(admin_nonnull_client *clt_dst,
virNetServerClientPtr clt_src)
{
clt_dst->id = virNetServerClientGetID(clt_src);
clt_dst->timestamp = virNetServerClientGetTimestamp(clt_src);
clt_dst->transport = virNetServerClientGetTransport(clt_src);
+ return 0;
}
/* Functions */
diff --git a/src/remote/remote_daemon_dispatch.c b/src/remote/remote_daemon_dispatch.c
index e62ebfb596..73a9434700 100644
--- a/src/remote/remote_daemon_dispatch.c
+++ b/src/remote/remote_daemon_dispatch.c
@@ -93,16 +93,16 @@ static virNWFilterPtr get_nonnull_nwfilter(virConnectPtr conn,
remote_nonnull_nw
static virNWFilterBindingPtr get_nonnull_nwfilter_binding(virConnectPtr conn,
remote_nonnull_nwfilter_binding binding);
static virDomainSnapshotPtr get_nonnull_domain_snapshot(virDomainPtr dom,
remote_nonnull_domain_snapshot snapshot);
static virNodeDevicePtr get_nonnull_node_device(virConnectPtr conn,
remote_nonnull_node_device dev);
-static void make_nonnull_domain(remote_nonnull_domain *dom_dst, virDomainPtr dom_src);
-static void make_nonnull_network(remote_nonnull_network *net_dst, virNetworkPtr
net_src);
-static void make_nonnull_interface(remote_nonnull_interface *interface_dst,
virInterfacePtr interface_src);
-static void make_nonnull_storage_pool(remote_nonnull_storage_pool *pool_dst,
virStoragePoolPtr pool_src);
-static void make_nonnull_storage_vol(remote_nonnull_storage_vol *vol_dst,
virStorageVolPtr vol_src);
-static void make_nonnull_node_device(remote_nonnull_node_device *dev_dst,
virNodeDevicePtr dev_src);
-static void make_nonnull_secret(remote_nonnull_secret *secret_dst, virSecretPtr
secret_src);
-static void make_nonnull_nwfilter(remote_nonnull_nwfilter *net_dst, virNWFilterPtr
nwfilter_src);
-static void make_nonnull_nwfilter_binding(remote_nonnull_nwfilter_binding *binding_dst,
virNWFilterBindingPtr binding_src);
-static void make_nonnull_domain_snapshot(remote_nonnull_domain_snapshot *snapshot_dst,
virDomainSnapshotPtr snapshot_src);
+static int make_nonnull_domain(remote_nonnull_domain *dom_dst, virDomainPtr dom_src)
ATTRIBUTE_RETURN_CHECK;
+static int make_nonnull_network(remote_nonnull_network *net_dst, virNetworkPtr net_src)
ATTRIBUTE_RETURN_CHECK;
+static int make_nonnull_interface(remote_nonnull_interface *interface_dst,
virInterfacePtr interface_src) ATTRIBUTE_RETURN_CHECK;
+static int make_nonnull_storage_pool(remote_nonnull_storage_pool *pool_dst,
virStoragePoolPtr pool_src) ATTRIBUTE_RETURN_CHECK;
+static int make_nonnull_storage_vol(remote_nonnull_storage_vol *vol_dst,
virStorageVolPtr vol_src) ATTRIBUTE_RETURN_CHECK;
+static int make_nonnull_node_device(remote_nonnull_node_device *dev_dst,
virNodeDevicePtr dev_src) ATTRIBUTE_RETURN_CHECK;
+static int make_nonnull_secret(remote_nonnull_secret *secret_dst, virSecretPtr
secret_src) ATTRIBUTE_RETURN_CHECK;
+static int make_nonnull_nwfilter(remote_nonnull_nwfilter *net_dst, virNWFilterPtr
nwfilter_src) ATTRIBUTE_RETURN_CHECK;
+static int make_nonnull_nwfilter_binding(remote_nonnull_nwfilter_binding *binding_dst,
virNWFilterBindingPtr binding_src) ATTRIBUTE_RETURN_CHECK;
+static int make_nonnull_domain_snapshot(remote_nonnull_domain_snapshot *snapshot_dst,
virDomainSnapshotPtr snapshot_src) ATTRIBUTE_RETURN_CHECK;
static int
remoteSerializeDomainDiskErrors(virDomainDiskErrorPtr errors,
@@ -315,7 +315,8 @@ remoteRelayDomainEventLifecycle(virConnectPtr conn,
/* build return data */
memset(&data, 0, sizeof(data));
- make_nonnull_domain(&data.dom, dom);
+ if (make_nonnull_domain(&data.dom, dom) < 0)
+ goto error;
data.event = event;
data.detail = detail;
@@ -335,6 +336,11 @@ remoteRelayDomainEventLifecycle(virConnectPtr conn,
}
return 0;
+
+ error:
+ xdr_free((xdrproc_t)xdr_remote_domain_event_lifecycle_msg,
+ &data);
+ return -1;
Strictly speaking, this is not needed. But I see the rationale behind.
ACK
Michal