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(a)redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/