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

Re: [Libvir] 1/4 Additions to the public API



On Thu, Feb 21, 2008 at 10:21:09PM +0000, Daniel P. Berrange wrote:
> On Thu, Feb 21, 2008 at 05:13:51PM -0500, Daniel Veillard wrote:
> > On Thu, Feb 21, 2008 at 08:56:17PM +0000, Richard W.M. Jones wrote:
> > > This just adds the four new functions to the public API.
> > [...]
> > > +/**
> > > + * virNetworkDHCPHostMapping:
> > > + *
> > > + * hostname mappings are returned by virNetworkListDHCPHostMapping.
> > > + */
> > > +typedef struct _virNetworkDHCPHostMapping {
> > > +  char *hwaddr;
> > > +  char *ipaddr;
> > > +  char *hostname;
> > > +} virNetworkDHCPHostMapping;
> > > +
> > > +typedef virNetworkDHCPHostMapping *virNetworkDHCPHostMappingPtr;
> > > +
> > [...]
> > > +int                     virNetworkListDHCPHostMappings
> > > +                                                (virNetworkPtr network,
> > > +				  virNetworkDHCPHostMappingPtr *const mappings,
> > > +			          int maxmappings);
> > > +int                     virNetworkFreeDHCPHostMappings
> > > +                                 (virNetworkDHCPHostMappingPtr *const mappings,
> > > +			          int maxmappings);
> > 
> >   Hum, do we really need to expose this structure at the API level, since we
> > use it only for listing. Since everything is provided as strings why not 
> > use something like
> > 
> >      int virNetworkListDHCPHostMappings (virNetworkPtr network,
> >                                          int maxmappings,
> >                                          char **hwaddrtab,
> > 					 char **ipaddrtab,
> > 					 char **hostnametab);
> > 
> >   This also avoids the free function too, same functionality, quite a simpler
> > interface.
> 
> Simpler for who ? Apps using this data now have to pass around a whole
> bunch of pointers, instead of a single virNetworkDHCPHostMapping. When 
> you map this API into higher level languages a struct is also going to be
> a much nicer thing to represent. 

if the struct was a simpler representation it would be used for Add too,
I don't think it's the case. Maybe the 3 arrays is not the nicest but 
defining a public structure holding 3 strings feels strange to me,
especially when in the other place we use the 3 strings directly.
Having a deallocator but no allocator function looks strange to me too.

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]