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/