[PATCH] admin: use g_autofree

[Peter Krempa]

On Mon, Mar 02, 2020 at 14:13:28 +0530, Gaurav Agrawal wrote:
> On review by Jan Tomko, for my earlier Patchv2, which had the return NULL

Please don't top-post on technical mailing lists.

> replace by the return conn ,

He told you the correct thing to do:

> here is the review:
> ```
> >+cleanup:
> >     return conn;

return conn MUST stay here. The function is returning a referenced 'conn
object on success.

> >
> >  error:
> >     virDispatchError(NULL);
> 
> >-    virObjectUnref(conn);
> 
> 'conn' is still not marked as g_auto, so this unref needs to stay.

On error 'conn' MUST be freed.

> 
> >     conn = NULL;
> >     goto cleanup;

And NULL must be returned on error. Since cleanup does nothing now other
than returning 'conn' which is NULL you can avoid the two lines by
returning NULL directly rather than jumping to 'cleanup'.

> 
> These two lines can be replaced by return NULL, and the cleanup: label
> above removed.

That's what's said here. There was no mention of changing what's
returned.

> 
> ```
> 
> I went ahead with what this review meant, and initially my PATCHv1 also did
> the same thing return NULL, because if there is an error the function is
> supposed to return NULL anyways.
> 
> 
> 
> On Mon, Mar 2, 2020 at 2:08 PM Peter Krempa <pkrempa at redhat.com> wrote:
> 
> > On Mon, Mar 02, 2020 at 07:22:03 +0530, Gaurav Agrawal wrote:
> > > Signed-off-by: Gaurav Agrawal <agrawalgaurav at gnome.org>
> > > ---
> > >  src/admin/libvirt-admin.c | 15 +++++----------
> > >  1 file changed, 5 insertions(+), 10 deletions(-)
> >
> > [...]
> >
> > > @@ -251,16 +251,11 @@ virAdmConnectOpen(const char *name, unsigned int
> > flags)
> > >      if (remoteAdminConnectOpen(conn, flags) < 0)
> > >          goto error;
> > >
> > > - cleanup:
> > > -    VIR_FREE(sock_path);
> > > -    VIR_FREE(uristr);
> > > -    return conn;
> >
> > Removing this 'return' will make this function ALWAYS return NULL when
> > the control flow now continues through the error label.
> >
> > >
> > >   error:
> > >      virDispatchError(NULL);
> > >      virObjectUnref(conn);
> > > -    conn = NULL;
> > > -    goto cleanup;
> > > +    return NULL;
> > >  }
> >
> >