Re: [libvirt] [PATCH 1/8] hellolibvirt: Update hellolibvirt example

On 19.02.2013 03:14, John Ferlan wrote: > Add a list of active domains, list of active/inactive networks, and > list of active/inactive storage pools > --- > examples/hellolibvirt/hellolibvirt.c | 201 ++++++++++++++++++++++++++++++----- > 1 file changed, 173 insertions(+), 28 deletions(-) > > diff --git a/examples/hellolibvirt/hellolibvirt.c b/examples/hellolibvirt/hellolibvirt.c > index 234637e..f191782 100644 > --- a/examples/hellolibvirt/hellolibvirt.c > +++ b/examples/hellolibvirt/hellolibvirt.c > @@ -85,65 +85,200 @@ out: > return ret; > } > > - > +typedef int (*virFunction)(virConnectPtr conn, > + char **nameList, > + int maxnames); > static int > -showDomains(virConnectPtr conn) > +listObject(virConnectPtr conn, int maxnames, const char *objNameStr, > + virFunction fcn) > { > - int ret = 0, i, numNames, numInactiveDomains, numActiveDomains; > + int ret = 0, i, numNames; > char **nameList = NULL; > > - numActiveDomains = virConnectNumOfDomains(conn); > - if (-1 == numActiveDomains) { > + nameList = malloc(sizeof(*nameList) * maxnames); > + > + if (NULL == nameList) { If blue is the sky .... These Yoda conditions should really be made inverted. But there are some even outside of the hellolibvirt.c. > ret = 1; > - printf("Failed to get number of active domains\n"); > + printf("Could not allocate memory for list of %s\n", objNameStr); > + goto out; > + } > + > + numNames = (*fcn)(conn, nameList, maxnames); > + > + if (-1 == numNames) { > + ret = 1; > + printf("Could not get list of %s from hypervisor\n", objNameStr); > showError(conn); > goto out; > } > > - numInactiveDomains = virConnectNumOfDefinedDomains(conn); > - if (-1 == numInactiveDomains) { > + printf(" %s: \n", objNameStr); > + for (i = 0; i < numNames; i++) { > + printf("\t%s\n", *(nameList + i)); > + /* The API documentation doesn't say so, but the names > + * returned by are strdup'd and must be freed here. > + */ The docs should be fixed then. > + free(*(nameList + i)); > + } > + > +out: > + free(nameList); > + return ret; > +} I wonder if we should advise users to use the other list APIs that are around for a while (virConnectListAll. I guess it all boils down to question if the hellolibvirt binary is supposed to work with ancient libvirts or is just an example shipped within a release. In case it is supposed to work with prehistoric versions, we must add a fallback code. If we are satisfied with the example working with current libvirt, there's no need for fallbacking code then.