[libvirt] [PATCH v2 2/2] Use virCheckFlags for APIs added in 0.8.0
Daniel Veillard
veillard at redhat.com
Thu Apr 15 14:50:36 UTC 2010
On Thu, Apr 15, 2010 at 08:41:07AM -0600, Eric Blake wrote:
> On 04/13/2010 10:25 AM, Jiri Denemark wrote:
> > ---
> > src/esx/esx_driver.c | 43 ++++++++++++-------------
> > src/nwfilter/nwfilter_driver.c | 4 ++-
> > src/qemu/qemu_driver.c | 68 +++++++++++++++++++---------------------
> > src/storage/storage_driver.c | 6 +---
> > src/vbox/vbox_tmpl.c | 41 ++++++++++++++++-------
> > src/xen/xend_internal.c | 4 ++
> > 6 files changed, 88 insertions(+), 78 deletions(-)
> >
> > diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
> > index 4ed9890..d9a1cfd 100644
> > --- a/src/esx/esx_driver.c
> > +++ b/src/esx/esx_driver.c
> > @@ -3296,7 +3296,7 @@ esxDomainIsPersistent(virDomainPtr domain ATTRIBUTE_UNUSED)
> >
> > static virDomainSnapshotPtr
> > esxDomainSnapshotCreateXML(virDomainPtr domain, const char *xmlDesc,
> > - unsigned int flags ATTRIBUTE_UNUSED)
> > + unsigned int flags)
> > {
> > esxPrivate *priv = domain->conn->privateData;
> > virDomainSnapshotDefPtr def = NULL;
> > @@ -3308,6 +3308,8 @@ esxDomainSnapshotCreateXML(virDomainPtr domain, const char *xmlDesc,
> > esxVI_TaskInfoState taskInfoState;
> > virDomainSnapshotPtr snapshot = NULL;
> >
> > + virCheckFlags(0, NULL);
>
> I scanned through the patch, and didn't see any instances where we are
> calling virCheckFlags after non-trivial work. It is something to be
> aware of when using the macro in the future, since:
>
> {
> ptr *foo = somethingThatMallocs();
> virCheckFlags(0, NULL);
>
> would be a memory leak, masked because the return is hidden inside the
> virCheckFlags macro.
>
> But the alternative, of making virCheckFlags result in a bool, and
> making all callers use:
>
> if (virBadFlags(0))
> return NULL;
>
> is not as aesthetic. So:
>
> ACK.
Agreed it's the right time, and internal.h is the right place for the
macro, AC :-)
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