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

Erik Skultety eskultet at redhat.com
Mon Jul 23 10:58:58 UTC 2018


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.

Erik




More information about the libvir-list mailing list