12:16 p.m.
On 12/28/2013 11:11 AM, Eric Blake wrote:
While auditing error messages in libvirt.c, I found a couple
instances that had not been converted to modern error styles,
and a few places that failed to dispatch the error through
the known-good connection.
* src/libvirt.c (virDomainPinEmulator, virDomainGetDiskErrors)
(virDomainSendKey, virDomainGetSecurityLabelList)
(virDomainGetEmulatorPinInfo): Use typical error reporting.
(virConnectGetCPUModelNames, virConnectRegisterCloseCallback)
(virConnectUnregisterCloseCallback, virDomainGetUUID): Report
error through connection.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
src/libvirt.c | 54 ++++++++++++++++++++++++++++--------------------------
1 file changed, 28 insertions(+), 26 deletions(-)
diff --git a/src/libvirt.c b/src/libvirt.c
index 815dd64..33f7078 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -3588,11 +3588,15 @@ virDomainGetUUID(virDomainPtr domain, unsigned char *uuid)
virDispatchError(NULL);
return -1;
}
- virCheckNonNullArgReturn(uuid, -1);
+ virCheckNonNullArgGoto(uuid, error);
memcpy(uuid, &domain->uuid[0], VIR_UUID_BUFLEN);
return 0;
+
+error:
+ virDispatchError(domain->conn);
+ return -1;
}
@@ -9866,11 +9870,14 @@ virDomainSendKey(virDomainPtr domain,
virResetLastError();
- if (keycodes == NULL ||
- nkeycodes <= 0 || nkeycodes > VIR_DOMAIN_SEND_KEY_MAX_KEYS) {
- virLibDomainError(VIR_ERR_OPERATION_INVALID, __FUNCTION__);
- virDispatchError(NULL);
- return -1;
+ virCheckNonNullArgGoto(keycodes, error);
+ virCheckPositiveArgGoto(nkeycodes, error);
+
+ if (nkeycodes > VIR_DOMAIN_SEND_KEY_MAX_KEYS) {
+ virReportInvalidArg(nkeycodes,
+ _("nkeycodes in %s must be less than %d"),
technically...
less than or equal to ...
+ __FUNCTION__,
VIR_DOMAIN_SEND_KEY_MAX_KEYS);
+ goto error;
}
I think the above code may need to move after the next two domain checks
(although looking forward, I realize it may be irrelevant)...
If domain is NULL at the goto error above, then the domain->conn deref
in error: will cause more havoc.
if (!VIR_IS_CONNECTED_DOMAIN(domain)) {
@@ -10477,10 +10484,8 @@ virDomainPinEmulator(virDomainPtr domain, unsigned char
*cpumap,
goto error;
}
- if ((cpumap == NULL) || (maplen < 1)) {
- virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__);
- goto error;
- }
+ virCheckNonNullArgGoto(cpumap, error);
+ virCheckPositiveArgGoto(maplen, error);
conn = domain->conn;
@@ -10537,15 +10542,15 @@ virDomainGetEmulatorPinInfo(virDomainPtr domain, unsigned char
*cpumap,
return -1;
}
- if (!cpumap || maplen <= 0) {
- virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__);
- goto error;
- }
+ virCheckNonNullArgGoto(cpumap, error);
+ virCheckPositiveArgGoto(maplen, error);
/* At most one of these two flags should be set. */
if ((flags & VIR_DOMAIN_AFFECT_LIVE) &&
(flags & VIR_DOMAIN_AFFECT_CONFIG)) {
- virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__);
+ virReportInvalidArg(flags,
+ _("flags 'affect live' and 'affect
config' in %s are mutually exclusive"),
+ __FUNCTION__);
The new error message is longer than 80 columns.. split across multiple
lines.
ACK with nits.
John
goto error;
}
conn = domain->conn;
@@ -10759,10 +10764,7 @@ virDomainGetSecurityLabelList(virDomainPtr domain,
return -1;
}
- if (seclabels == NULL) {
- virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__);
- goto error;
- }
+ virCheckNonNullArgGoto(seclabels, error);
conn = domain->conn;
@@ -18815,7 +18817,7 @@ virConnectGetCPUModelNames(virConnectPtr conn, const char *arch,
char ***models,
virDispatchError(NULL);
return -1;
}
- virCheckNonNullArgReturn(arch, -1);
+ virCheckNonNullArgGoto(arch, error);
if (conn->driver->connectGetCPUModelNames) {
int ret;
@@ -21887,10 +21889,10 @@ virConnectRegisterCloseCallback(virConnectPtr conn,
return 0;
error:
+ virDispatchError(conn);
virObjectUnlock(conn->closeCallback);
virMutexUnlock(&conn->lock);
virObjectUnref(conn);
- virDispatchError(NULL);
return -1;
}
@@ -21945,9 +21947,9 @@ virConnectUnregisterCloseCallback(virConnectPtr conn,
return 0;
error:
+ virDispatchError(conn);
virObjectUnlock(conn->closeCallback);
virMutexUnlock(&conn->lock);
- virDispatchError(NULL);
return -1;
}
@@ -22305,10 +22307,10 @@ virDomainGetDiskErrors(virDomainPtr dom,
return -1;
}
- if ((!errors && maxerrors) || (errors && !maxerrors)) {
- virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
- goto error;
- }
+ if (maxerrors)
+ virCheckNonNullArgGoto(errors, error);
+ else
+ virCheckNullArgGoto(errors, error);
if (dom->conn->driver->domainGetDiskErrors) {
int ret = dom->conn->driver->domainGetDiskErrors(dom, errors,