[libvirt] [PATCH v1 01/40] util: cgroup: modify virCgroupFree to take virCgroupPtr

[Sukrit Bhatnagar]

On Mon, 23 Jul 2018 at 16:29, Erik Skultety <eskultet at redhat.com> wrote:
>
> On Sat, Jul 21, 2018 at 05:36:33PM +0530, Sukrit Bhatnagar wrote:
> > Modify virCgroupFree function signature to take a value of type
> > virCgroupPtr instead of virCgroupPtr * as the parameter.
> >
> > Change the argument type in all calls to virCgroupFree function
> > from virCgroupPtr * to virCgroupPtr.
>
> ^This sentence doesn't add any useful information. Instead, the commit message
> should add information about why we're performing this change, i.e. in order to
> enable usage of VIR_AUTOPTR with cgroup module or something alike.
> Also, this patch is oddly placed, IMHO it should come before patch 8, where the
> other work on cgroup module is done.
>
> With that:
> Reviewed-by: Erik Skultety <eskultet at redhat.com>
>
> ...
>
> > @@ -1379,8 +1379,8 @@ virCgroupNewPartition(const char *path,
> >      ret = 0;
> >   cleanup:
> >      if (ret != 0)
> > -        virCgroupFree(group);
> > -    virCgroupFree(&parent);
> > +        virCgroupFree(*group);
> > +    virCgroupFree(parent);
>
> Since you're already touching the code, I'd appreciate another "adjustment"
> patch where occurrences like ^this will be replaced by a VIR_STEAL_PTR, IOW
> where we're passing a reference to a pointer in order to change the original
> pointer, we should use a VIR_STEAL_PTR just before the cleanup section, so that
> we don't need an if (ret != 0) or if (ret < 0) check only to conditionally do
> some cleanup. Feel free to let me know if none of what I just wrote is clear.

I am assuming that you are referring to `group` variable. If so, then I cannot
apply cleanup attribute to function parameters and `group` is one of them.