[libvirt] [PATCH] Improve invalid argument checks for the public API
Daniel Veillard
veillard at redhat.com
Tue May 17 13:14:06 UTC 2011
On Tue, May 17, 2011 at 02:45:44PM +0200, Matthias Bolte wrote:
> ---
> src/libvirt.c | 100 +++++++++++++++++++++++++++++++++++++++++++++++++++++---
> 1 files changed, 94 insertions(+), 6 deletions(-)
Damn, that's a lot of missing checks or not ideal reports
> diff --git a/src/libvirt.c b/src/libvirt.c
> index 787908e..87391c6 100644
> --- a/src/libvirt.c
> +++ b/src/libvirt.c
> @@ -1390,7 +1390,7 @@ int
> virConnectRef(virConnectPtr conn)
> {
> if ((!VIR_IS_CONNECT(conn))) {
> - virLibConnError(VIR_ERR_INVALID_ARG, __FUNCTION__);
> + virLibConnError(VIR_ERR_INVALID_CONN, __FUNCTION__);
> virDispatchError(NULL);
> return -1;
> }
> @@ -4487,7 +4487,6 @@ virDomainMigratePrepareTunnel(virConnectPtr conn,
> const char *dname,
> unsigned long bandwidth,
> const char *dom_xml)
> -
> {
> VIR_DEBUG("conn=%p, stream=%p, flags=%lu, dname=%s, "
> "bandwidth=%lu, dom_xml=%s", conn, st, flags,
> @@ -4994,6 +4993,12 @@ virDomainGetSchedulerType(virDomainPtr domain, int *nparams)
> virDispatchError(NULL);
> return NULL;
> }
> +
> + if (nparams == NULL) {
> + virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__);
> + goto error;
> + }
> +
> conn = domain->conn;
>
> if (conn->driver->domainGetSchedulerType){
> @@ -5040,6 +5045,12 @@ virDomainGetSchedulerParameters(virDomainPtr domain,
> virDispatchError(NULL);
> return -1;
> }
> +
> + if (params == NULL || nparams == NULL) {
> + virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__);
> + goto error;
> + }
I was just wondering here if params being NULL couldn't be used to
find out the correct value for nparams, but looking at least at the
qemu driver code we really expect the array there.
> conn = domain->conn;
>
> if (conn->driver->domainGetSchedulerParameters) {
> @@ -5084,6 +5095,12 @@ virDomainSetSchedulerParameters(virDomainPtr domain,
> virDispatchError(NULL);
> return -1;
> }
> +
> + if (params == NULL) {
> + virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__);
> + goto error;
> + }
> +
> if (domain->conn->flags & VIR_CONNECT_RO) {
> virLibDomainError(VIR_ERR_OPERATION_DENIED, __FUNCTION__);
> goto error;
> @@ -5534,7 +5551,7 @@ virDomainGetBlockInfo(virDomainPtr domain, const char *path, virDomainBlockInfoP
> virDispatchError(NULL);
> return -1;
> }
> - if (info == NULL) {
> + if (path == NULL || info == NULL) {
> virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__);
> goto error;
> }
> @@ -6440,6 +6457,12 @@ virDomainAttachDevice(virDomainPtr domain, const char *xml)
> virDispatchError(NULL);
> return -1;
> }
> +
> + if (xml == NULL) {
> + virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__);
> + goto error;
> + }
> +
> if (domain->conn->flags & VIR_CONNECT_RO) {
> virLibDomainError(VIR_ERR_OPERATION_DENIED, __FUNCTION__);
> goto error;
> @@ -6500,6 +6523,12 @@ virDomainAttachDeviceFlags(virDomainPtr domain,
> virDispatchError(NULL);
> return -1;
> }
> +
> + if (xml == NULL) {
> + virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__);
> + goto error;
> + }
> +
> if (domain->conn->flags & VIR_CONNECT_RO) {
> virLibDomainError(VIR_ERR_OPERATION_DENIED, __FUNCTION__);
> goto error;
> @@ -6545,6 +6574,12 @@ virDomainDetachDevice(virDomainPtr domain, const char *xml)
> virDispatchError(NULL);
> return -1;
> }
> +
> + if (xml == NULL) {
> + virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__);
> + goto error;
> + }
> +
> if (domain->conn->flags & VIR_CONNECT_RO) {
> virLibDomainError(VIR_ERR_OPERATION_DENIED, __FUNCTION__);
> goto error;
> @@ -6601,6 +6636,12 @@ virDomainDetachDeviceFlags(virDomainPtr domain,
> virDispatchError(NULL);
> return -1;
> }
> +
> + if (xml == NULL) {
> + virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__);
> + goto error;
> + }
> +
> if (domain->conn->flags & VIR_CONNECT_RO) {
> virLibDomainError(VIR_ERR_OPERATION_DENIED, __FUNCTION__);
> goto error;
> @@ -6661,6 +6702,12 @@ virDomainUpdateDeviceFlags(virDomainPtr domain,
> virDispatchError(NULL);
> return -1;
> }
> +
> + if (xml == NULL) {
> + virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__);
> + goto error;
> + }
> +
> if (domain->conn->flags & VIR_CONNECT_RO) {
> virLibDomainError(VIR_ERR_OPERATION_DENIED, __FUNCTION__);
> goto error;
> @@ -7321,7 +7368,7 @@ int
> virNetworkRef(virNetworkPtr network)
> {
> if ((!VIR_IS_CONNECTED_NETWORK(network))) {
> - virLibConnError(VIR_ERR_INVALID_ARG, __FUNCTION__);
> + virLibConnError(VIR_ERR_INVALID_NETWORK, __FUNCTION__);
> virDispatchError(NULL);
> return -1;
> }
> @@ -8187,7 +8234,7 @@ int
> virInterfaceRef(virInterfacePtr iface)
> {
> if ((!VIR_IS_CONNECTED_INTERFACE(iface))) {
> - virLibConnError(VIR_ERR_INVALID_ARG, __FUNCTION__);
> + virLibConnError(VIR_ERR_INVALID_INTERFACE, __FUNCTION__);
> virDispatchError(NULL);
> return -1;
> }
> @@ -8630,7 +8677,7 @@ virStoragePoolLookupByVolume(virStorageVolPtr vol)
> virResetLastError();
>
> if (!VIR_IS_CONNECTED_STORAGE_VOL(vol)) {
> - virLibConnError(VIR_ERR_INVALID_CONN, __FUNCTION__);
> + virLibConnError(VIR_ERR_INVALID_STORAGE_VOL, __FUNCTION__);
> virDispatchError(NULL);
> return NULL;
> }
> @@ -9702,6 +9749,11 @@ virStorageVolCreateXML(virStoragePoolPtr pool,
> return NULL;
> }
>
> + if (xmldesc == NULL) {
> + virLibConnError(VIR_ERR_INVALID_ARG, __FUNCTION__);
> + goto error;
> + }
> +
> if (pool->conn->flags & VIR_CONNECT_RO) {
> virLibConnError(VIR_ERR_OPERATION_DENIED, __FUNCTION__);
> goto error;
> @@ -9758,6 +9810,11 @@ virStorageVolCreateXMLFrom(virStoragePoolPtr pool,
> goto error;
> }
>
> + if (xmldesc == NULL) {
> + virLibConnError(VIR_ERR_INVALID_ARG, __FUNCTION__);
> + goto error;
> + }
> +
> if (pool->conn->flags & VIR_CONNECT_RO ||
> clonevol->conn->flags & VIR_CONNECT_RO) {
> virLibConnError(VIR_ERR_OPERATION_DENIED, __FUNCTION__);
> @@ -10508,6 +10565,11 @@ int virNodeDeviceListCaps(virNodeDevicePtr dev,
> return -1;
> }
>
> + if (names == NULL || maxnames < 0) {
> + virLibNodeDeviceError(VIR_ERR_INVALID_ARG, __FUNCTION__);
> + goto error;
> + }
> +
> if (dev->conn->deviceMonitor && dev->conn->deviceMonitor->deviceListCaps) {
> int ret;
> ret = dev->conn->deviceMonitor->deviceListCaps (dev, names, maxnames);
> @@ -11764,6 +11826,11 @@ int virStreamSend(virStreamPtr stream,
> return -1;
> }
>
> + if (data == NULL) {
> + virLibConnError(VIR_ERR_INVALID_ARG, __FUNCTION__);
> + goto error;
> + }
> +
> if (stream->driver &&
> stream->driver->streamSend) {
> int ret;
> @@ -11859,6 +11926,11 @@ int virStreamRecv(virStreamPtr stream,
> return -1;
> }
>
> + if (data == NULL) {
> + virLibConnError(VIR_ERR_INVALID_ARG, __FUNCTION__);
> + goto error;
> + }
> +
> if (stream->driver &&
> stream->driver->streamRecv) {
> int ret;
> @@ -11935,6 +12007,11 @@ int virStreamSendAll(virStreamPtr stream,
> return -1;
> }
>
> + if (handler == NULL) {
> + virLibConnError(VIR_ERR_INVALID_ARG, __FUNCTION__);
> + goto cleanup;
> + }
> +
> if (stream->flags & VIR_STREAM_NONBLOCK) {
> virLibConnError(VIR_ERR_OPERATION_INVALID,
> _("data sources cannot be used for non-blocking streams"));
> @@ -12032,6 +12109,11 @@ int virStreamRecvAll(virStreamPtr stream,
> return -1;
> }
>
> + if (handler == NULL) {
> + virLibConnError(VIR_ERR_INVALID_ARG, __FUNCTION__);
> + goto cleanup;
> + }
> +
> if (stream->flags & VIR_STREAM_NONBLOCK) {
> virLibConnError(VIR_ERR_OPERATION_INVALID,
> _("data sinks cannot be used for non-blocking streams"));
> @@ -13746,6 +13828,12 @@ virDomainSnapshotCreateXML(virDomainPtr domain,
> }
>
> conn = domain->conn;
> +
> + if (xmlDesc == NULL) {
> + virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__);
> + goto error;
> + }
> +
> if (conn->flags & VIR_CONNECT_RO) {
> virLibConnError(VIR_ERR_OPERATION_DENIED, __FUNCTION__);
> goto error;
ACK, seems all those check are better handled directly in the front end
function
thanks !
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
daniel at veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
More information about the libvir-list
mailing list