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@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