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@xxxxxxxxxx>
> ---
> 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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
