[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [[PATCH libvirt-java]] Implement Connect.listAllDomains



Hello Claudio,

On Fri, 2014-10-31 at 23:40 +0100, Claudio Bley wrote:
> At Sat, 25 Oct 2014 16:25:48 -0700,
> C├ędric Bosdonnat wrote:
> I'd prefer an enum instead of these (ugly) constants.
> 
> As a side node, these constants are useless since the
> ListAllDomainsFlags is not public.

Ok, I mimicked similar code in the Domain class... Just changed it to
use an enum.

> >      /**
> >       * Get the version of a connection.
> >       *
> > @@ -758,6 +781,29 @@ public class Connect {
> >      }
> >  
> >      /**
> > +     * Lists a possibly filtered list of all the domains.
> > +     * 
> > +     * @param flags bitwise-OR of ListAllDomainsFlags
> > +     *
> > +     * @return and array of the IDs of the active domains
> > +     * @throws LibvirtException
> > +     */
> > +    public Domain[] listAllDomains(int flags) throws LibvirtException {
> > +        PointerByReference domainsRef = new PointerByReference();
> > +        int ret = libvirt.virConnectListAllDomains(VCP, domainsRef, flags);
> > +        processError(ret);
> > +
> > +        Pointer[] pointers  = domainsRef.getValue().getPointerArray(0);
> > +        Domain[] domains = new Domain[ret];
> > +        for (int i = 0; i < ret; i++) {
> > +            DomainPointer domainPtr = new DomainPointer();
> > +            domainPtr.setPointer(pointers[i]);
> > +            domains[i] = new Domain(this, domainPtr);
> > +        }
> > +        return domains;
> > +    }
> 
> You leak the memory of the array here.

Arf, with all those Java things, I forgot about the hidden C pointers
behind. I guess a

Library.free(domainsRef.getValue());

inside a finally block would do the trick.

In your other mail you mention freeing the domains pointers, but the
only method that could throw an exception is virConnectListAllDomains...
unless we need to catch also any other exception like those thrown by a
new.

--
Cedric


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]