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

[Tim Wiederhake]

Hi Ján!

Am Montag, den 07.09.2020, 17:10 +0200 schrieb Ján Tomko:
> On a Monday in 2020, Tim Wiederhake wrote:
> > Signed-off-by: Tim Wiederhake <twiederh at redhat.com>
> > ---
> > 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

Thanks for the explanation! And you are absolutely right about the
define. I did use g_autofree in a later patch, so I cannot really tell
why I thought the define was necessary here. Fixed (locally).

Tim

> > 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
> >