8:34 a.m.
On Fri, May 25, 2012 at 05:35:20PM -0600, Eric Blake wrote:
On 05/25/2012 11:44 AM, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange(a)redhat.com>
>
> To ensure consistent error reporting of invalid arguments,
> provide a number of predefined helper methods & macros.
>
> - An arg which must not be NULL:
>
> virCheckNonNullArgReturn(argname, retvalue)
> virCheckNonNullArgGoto(argname, label)
... Looks useful. I'll review the macros first.
[snip]
ACK with problems fixed. If you post a v2, post an interdiff (I
don't
want to scroll through the whole thing again :)
Here is what I propose to amend before pushing
diff --git a/src/internal.h b/src/internal.h
index 825b0fe..1b1598b 100644
--- a/src/internal.h
+++ b/src/internal.h
@@ -232,7 +232,7 @@
do { \
unsigned long __unsuppflags = flags & ~(supported); \
if (__unsuppflags) { \
- virReportInvalidArg("flags", \
+ virReportInvalidArg(flags, \
_("unsupported flags (0x%lx) in function %s"),
\
__unsuppflags, __FUNCTION__); \
return retval; \
@@ -242,51 +242,51 @@
# define virCheckNonNullArgReturn(argname, retval) \
do { \
if (argname == NULL) { \
- virReportInvalidNullArg(#argname); \
+ virReportInvalidNullArg(argname); \
return retval; \
} \
} while (0)
# define virCheckNullArgGoto(argname, label) \
do { \
if (argname == NULL) { \
- virReportInvalidNullArg(#argname); \
+ virReportInvalidNullArg(argname); \
goto label; \
} \
} while (0)
# define virCheckNonNullArgGoto(argname, label) \
do { \
if (argname == NULL) { \
- virReportInvalidNonNullArg(#argname); \
+ virReportInvalidNonNullArg(argname); \
goto label; \
} \
} while (0)
# define virCheckPositiveArgGoto(argname, label) \
do { \
if (argname <= 0) { \
- virReportInvalidPositiveArg(#argname); \
+ virReportInvalidPositiveArg(argname); \
goto label; \
} \
} while (0)
# define virCheckNonZeroArgGoto(argname, label) \
do { \
if (argname == 0) { \
- virReportInvalidNonZeroArg(#argname); \
+ virReportInvalidNonZeroArg(argname); \
goto label; \
} \
} while (0)
# define virCheckZeroArgGoto(argname, label) \
do { \
if (argname != 0) { \
- virReportInvalidNonZeroArg(#argname); \
+ virReportInvalidNonZeroArg(argname); \
goto label; \
} \
} while (0)
-# define virCheckNonNegativeArgGoto(argname, label) \
- do { \
- if (argname < 0) { \
- virReportInvalidNonNegativeArg(#argname); \
- goto label; \
- } \
+# define virCheckNonNegativeArgGoto(argname, label) \
+ do { \
+ if (argname < 0) { \
+ virReportInvalidNonNegativeArg(argname); \
+ goto label; \
+ } \
} while (0)
diff --git a/src/libvirt.c b/src/libvirt.c
index d52557e..2fc85f0 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -4918,16 +4918,16 @@ virDomainMigratePeer2Peer (virDomainPtr domain,
}
if (!tempuri->server) {
- virReportInvalidArg(tempuri,
- _("server field in tempuri in %s must not be
NULL"),
+ virReportInvalidArg(dconnuri,
+ _("unable to parser server from dconnuri in %s"),
__FUNCTION__);
virDispatchError(domain->conn);
virURIFree(tempuri);
return -1;
}
if (STRPREFIX(tempuri->server, "localhost")) {
- virReportInvalidArg(tempuri,
- _("server field in tempuri in %s must not be
'localhost'"),
+ virReportInvalidArg(dconnuri,
+ _("unable to parser server from dconnuri in %s"),
__FUNCTION__);
virDispatchError(domain->conn);
virURIFree(tempuri);
@@ -7041,7 +7041,7 @@ virDomainBlockStats(virDomainPtr dom, const char *disk,
virCheckNonNullArgGoto(stats, error);
if (size > sizeof(stats2)) {
virReportInvalidArg(size,
- _("size in %s must be less than %zu"),
+ _("size in %s must not exceed %zu"),
__FUNCTION__, sizeof(stats2));
goto error;
}
@@ -7186,7 +7186,7 @@ virDomainInterfaceStats (virDomainPtr dom, const char *path,
virCheckNonNullArgGoto(stats, error);
if (size > sizeof(stats2)) {
virReportInvalidArg(size,
- _("size in %s must be less than %zu"),
+ _("size in %s must not exceed %zu"),
__FUNCTION__, sizeof(stats2));
goto error;
}
@@ -14150,7 +14150,7 @@ virSecretGetUUID(virSecretPtr secret, unsigned char *uuid)
return 0;
error:
- virDispatchError(NULL);
+ virDispatchError(secret->conn);
return -1;
}
@@ -16844,21 +16844,21 @@ virDomainSnapshotCreateXML(virDomainPtr domain,
if ((flags & VIR_DOMAIN_SNAPSHOT_CREATE_CURRENT) &&
!(flags & VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE)) {
virReportInvalidArg(flags,
- _("use of current flag in %s requires redefine
flag"),
+ _("use of 'current' flag in %s requires
'redefine' flag"),
__FUNCTION__);
goto error;
}
if ((flags & VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE) &&
(flags & VIR_DOMAIN_SNAPSHOT_CREATE_NO_METADATA)) {
virReportInvalidArg(flags,
- _("redefine and no metadata flags in %s are mutually
exclusive"),
+ _("'redefine' and 'no metadata' flags in
%s are mutually exclusive"),
__FUNCTION__);
goto error;
}
if ((flags & VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE) &&
(flags & VIR_DOMAIN_SNAPSHOT_CREATE_HALT)) {
virReportInvalidArg(flags,
- _("redefine and halt flags in %s are mutually
exclusive"),
+ _("'redefine' and 'halt' flags in %s are
mutually exclusive"),
__FUNCTION__);
goto error;
}
@@ -18406,11 +18406,15 @@ int virDomainGetCPUStats(virDomainPtr domain,
* ncpus must be non-zero unless params == NULL
* nparams * ncpus must not overflow (RPC may restrict it even more)
*/
- if (start_cpu < -1 && ncpus != -1) {
- virReportInvalidArg(start_cpu,
- _("start_cpu in %s must be -1 or greater if ncpus is not
-1"),
- __FUNCTION__);
- goto error;
+ if (start_cpu == -1) {
+ if (ncpus != 1) {
+ virReportInvalidArg(start_cpu,
+ _("ncpus in %s must be 1 when start_cpu is
-1"),
+ __FUNCTION__);
+ goto error;
+ }
+ } else {
+ virCheckNonNegativeArgGoto(start_cpu, error);
}
if (nparams)
virCheckNonNullArgGoto(params, error);
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|