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