Re: [libvirt] [PATCH v2 10/41] util: cgroup: use VIR_AUTOPTR for aggregate types
On Tue, Jul 24, 2018 at 09:22:11PM +0530, Sukrit Bhatnagar wrote:
By making use of GNU C's cleanup attribute handled by the
VIR_AUTOPTR macro for declaring aggregate pointer variables,
majority of the calls to *Free functions can be dropped, which
in turn leads to getting rid of most of our cleanup sections.
Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com>
Reviewed-by: Erik Skultety <eskultet(a)redhat.com>
---
...
@@ -1277,10 +1268,10 @@ virCgroupNewPartition(const char *path,
int controllers,
virCgroupPtr *group)
{
- int ret = -1;
VIR_AUTOFREE(char *) parentPath = NULL;
VIR_AUTOFREE(char *) newPath = NULL;
- virCgroupPtr parent = NULL;
+ VIR_AUTOPTR(virCgroup) parent = NULL;
+ VIR_AUTOPTR(virCgroup) tmpGroup = NULL;
VIR_DEBUG("path=%s create=%d controllers=%x",
path, create, controllers);
@@ -1315,12 +1306,11 @@ virCgroupNewPartition(const char *path,
}
}
- ret = 0;
+ return 0;
+
cleanup:
- if (ret != 0)
- virCgroupFree(*group);
- virCgroupFree(parent);
- return ret;
+ VIR_STEAL_PTR(tmpGroup, *group);
+ return -1;
^Not exactly what I had in mind, we're still touching the caller provided data
too much. Have a look at this diff:
diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
index 61fafe26f8..472a8167f5 100644
--- a/src/util/vircgroup.c
+++ b/src/util/vircgroup.c
@@ -1283,34 +1283,31 @@ virCgroupNewPartition(const char *path,
}
if (virCgroupSetPartitionSuffix(path, &newPath) < 0)
- goto cleanup;
+ return -1;
- if (virCgroupNew(-1, newPath, NULL, controllers, group) < 0)
- goto cleanup;
+ if (virCgroupNew(-1, newPath, NULL, controllers, &tmpGroup) < 0)
+ return -1;
if (STRNEQ(newPath, "/")) {
char *tmp;
if (VIR_STRDUP(parentPath, newPath) < 0)
- goto cleanup;
+ return -1;
tmp = strrchr(parentPath, '/');
tmp++;
*tmp = '\0';
if (virCgroupNew(-1, parentPath, NULL, controllers, &parent) < 0)
- goto cleanup;
+ return -1;
- if (virCgroupMakeGroup(parent, *group, create, VIR_CGROUP_NONE) < 0) {
- virCgroupRemove(*group);
- goto cleanup;
+ if (virCgroupMakeGroup(parent, tmpGroup, create, VIR_CGROUP_NONE) < 0) {
+ virCgroupRemove(tmpGroup);
+ return -1;
}
}
+ VIR_STEAL_PTR(*group, tmpGroup);
return 0;
-
- cleanup:
- VIR_STEAL_PTR(tmpGroup, *group);
- return -1;
After ^this, we'll only touch the caller provided data once we're sure nothing
can go wrong anymore. I'll squash that in.
Erik