Re: [libvirt PATCH 01/15] cpu_map: Use g_auto* in loadData

On a Monday in 2020, Tim Wiederhake wrote: > Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com&gt; > --- > src/cpu/cpu_map.c | 20 ++++++++------------ > 1 file changed, 8 insertions(+), 12 deletions(-) > > diff --git a/src/cpu/cpu_map.c b/src/cpu/cpu_map.c > index cbf90d1395..c315ab32b2 100644 > --- a/src/cpu/cpu_map.c > +++ b/src/cpu/cpu_map.c > @@ -32,6 +32,8 @@ > > VIR_LOG_INIT("cpu.cpu_map"); > > +G_DEFINE_AUTOPTR_CLEANUP_FUNC(xmlNodePtr, g_free); > + To answer the question from the cover letter: For internal types, we put the G_DEFINE_AUTO in the header files introducing the particular type. For types from external libraries (like libxml2), we put them into the header file with our wrappers over it. For public libvirt types, IIRC we only have defined the cleanup functions privately in virsh (tools/virsh-util.h) And this one does not need the define, we simply use g_autofree for types that are freed by g_free. Jano

> static int > loadData(const char *mapfile, > xmlXPathContextPtr ctxt, > @@ -39,20 +41,19 @@ loadData(const char *mapfile, > cpuMapLoadCallback callback, > void *data) > { > - int ret = -1; > VIR_XPATH_NODE_AUTORESTORE(ctxt) > - xmlNodePtr *nodes = NULL; > + g_autoptr(xmlNodePtr) nodes = NULL; > int n; > size_t i; > int rv; > > if ((n = virXPathNodeSet(element, ctxt, &nodes)) < 0) > - goto cleanup; > + return -1; > > if (n > 0 && !callback) { > virReportError(VIR_ERR_INTERNAL_ERROR, > _("Unexpected element '%s' in CPU map > '%s'"), element, mapfile); > - goto cleanup; > + return -1; > } > > for (i = 0; i < n; i++) { > @@ -60,22 +61,17 @@ loadData(const char *mapfile, > if (!name) { > virReportError(VIR_ERR_INTERNAL_ERROR, > _("cannot find %s name in CPU map > '%s'"), element, mapfile); > - goto cleanup; > + return -1; > } > VIR_DEBUG("Load %s name %s", element, name); > ctxt->node = nodes[i]; > rv = callback(ctxt, name, data); > VIR_FREE(name); > if (rv < 0) > - goto cleanup; > + return -1; > } > > - ret = 0; > - > - cleanup: > - VIR_FREE(nodes); > - > - return ret; > + return 0; > } > > static int > -- > 2.26.2 >