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

Re: [Libvir] PATCH: 5/10: public auth callback API



On Tue, Dec 04, 2007 at 03:50:46PM +0000, Richard W.M. Jones wrote:
> Daniel Veillard wrote:
> >On Tue, Dec 04, 2007 at 12:57:39PM +0000, Richard W.M. Jones wrote:
> >>Daniel P. Berrange wrote:
> >>>virConnectPtr           virConnectOpenAuth      (const char *name,
> >>>						 virConnectAuthPtr auth,
> >>>						 int flags);
> >>I'm a fan of callers passing in the size of the structure (as they see 
> >>it).  Allows the structure to be expanded in future, and if done right 
> >>can allow both forwards and backwards compatibility.
> >>
> >>cf: 
> >>http://www.libvirt.org/html/libvirt-libvirt.html#virDomainInterfaceStats
> >
> >  Hum, honnestly, that's not my preferred way. If you really think there
> >should be room for expansion, I would either:
> >   - add a version number to the structure and allocator/destructor
> >     functions as part of the API (prefered)
> 
> Hmmm ... what if they forget to set it?  Now we need an init function 
> for the structure.

   yes that's why I suggested an allocator, it would set the version.

> >   - add padding at tyhe end of the structure which could allow 
> >     a future growth
> 
> This incurs an unnecessary memory penalty always, and limits us to a 
> particular maximum size which we must choose now when we don't know what 
> extra fields we might add.

  I don't like this much either, but I think the GNOME guys did this
for libxml2 I usually provide allocator/deallocators.

> >adding the size of the structure as the argument moves the complexity away
> >from the library implementor to the library user, 
> 
> Well, it means they must append "sizeof (*auth)" to their list of 
> arguments, which is hardly a lot of complexity.  There _is_ much more 

  until they just add a numeric value ... and that will break just as
well. trying to minimize the cases where the API user can do the wrong
thing is an important goal :-) . There that's something which seems to
work but will break later, that's why I prefer an allocator if we plan to
change sizes.

> Remember also the need for forwards compatibility: program which was 
> compiled against an old version of the library, dynamically links to a 
> new version of the library.  If the structure has increased in size in 
> the meantime, then having the caller pass the size means that the 
> library can do the right thing and only fill in the first fields.

  The version + allocator have that property too, except it's the library
code which takes care of things.

Daniel

-- 
Red Hat Virtualization group http://redhat.com/virtualization/
Daniel Veillard      | virtualization library  http://libvirt.org/
veillard redhat com  | libxml GNOME XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine  http://rpmfind.net/


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