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

Re: [libvirt] [PATCH 1/5] lib: Add public api to enable atomic listing of guest



On Mon, May 21, 2012 at 04:42:41PM -0600, Eric Blake wrote:
> > +int                     virConnectListAllDomains (virConnectPtr conn,
> > +                                                  virDomainPtr **domains,
> 
> Hmm - time for me to think out loud.  Your signature differs from mine.
>  I proposed:
> 
> virDomainPtr *domains
> aka
> virDomain **domains
> 
> where my return value would be an array of virDomain objects, and the
> caller would have to do:
> 
> virDomainPtr dom, first;
> virConnectListAllDomains(conn, &first, flags);
> dom = first;
> while (dom) {
>    use dom;
>    virDomainFree(dom);
>    dom++;
> }
> free(first);
> 
> But that would only work if virDomain is not an opaque type.
> 
> You proposed:
> 
> virDomainPtr **domains
> aka
> virDomain ***domains
> 
> where the return is an array of virDomainPtr pointers.  The caller would
> then do:
> 
> virDomainPtr *array;
> virDomainPtr dom;
> int i = 0;
> virConnectListAllDomains(conn, &array, flags);
> dom = array[0];
> do {
>    use dom;
>    virDomainFree(dom);
>    dom = array[i++];
> } while (dom);
> free(array);
> 
> (Of course, you could use a for (i=0; i < ret; i++) loop instead of a
> while loop; I chose that style to constrast the difference between the
> number of pointer dereferences needed).  In conclusion, I think your
> style is right.  But it also means that we ought to document a sample
> usage as part of the API call :)

I agree that Michal's style is nicer & of course docs are always great

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 :|


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