On 7/10/19 6:50 AM, Eric Blake wrote:
On 7/10/19 2:02 AM, Peter Krempa wrote:
On Tue, Jul 09, 2019 at 12:46:30 -0500, Eric Blake wrote:
Even though we don't accept any flags, it is unfriendly to callers that use the modern API to have to fall back to the flag-free API.
Signed-off-by: Eric Blake <eblake@redhat.com> ---
-vboxDomainSave(virDomainPtr dom, const char *path ATTRIBUTE_UNUSED) +vboxDomainSaveFlags(virDomainPtr dom, const char *path ATTRIBUTE_UNUSED, + const char *dxml, unsigned int flags) { vboxDriverPtr data = dom->conn->privateData; IConsole *console = NULL; @@ -564,6 +565,9 @@ vboxDomainSave(virDomainPtr dom, const char *path ATTRIBUTE_UNUSED) nsresult rc; int ret = -1;
+ virCheckFlags(0, -1); + virCheckNonNullArgReturn(dxml, -1);
This reports: invalid argument: dxml in vboxDomainSave must not be NULL
Revisiting this thread: internal.h has: virCheckNullArgGoto (complains if argument is not NULL) virCheckNonNullArgReturn (complains if argument is NULL) virCheckNonNullArgGoto (complains if argument is NULL) but is missing virCheckNullArgReturn, which is the form I really meant to use here. I have no idea if that missing macro is intentional, but it would be easy enough to add.
I'm not certain that the internal function name makes sense to external users.
In which case I can hand-roll a more specific error instead of reusing the common macro. But I do see pre-existing uses of virCheckNonNullArgXXX in src/esx and src/vz prior to this patch, so it's not the first time we've used the common macros.
Directly calling virReportInvalidNonNullArg() would be the only way to hand-roll this properly, but no one outside of internal.h and src/util/virobject.c uses it. I'd prefer to sick with virCheckNullArgReturn().
+static int +vboxDomainSave(virDomainPtr dom, const char *path) +{ + return vboxDomainSaveFlags(dom, path, NULL, 0);
So, this passes NULL 'dxml' into vboxDomainSaveFlags which explicitly rejects it. What's the point?
Ah I see. Was the above supposed to be virCheckNullArgGoto?
D'oh. Yes, I got it backwards. The function wants to succeed only if the user omitted the optional dxml argument.
-- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org