Re: [libvirt] [PATCH v1 24/32] util: numa: use VIR_AUTOFREE instead of VIR_FREE for scalar types

2018-08-06 Thread Erik Skultety 
On Sat, Jul 28, 2018 at 11:31:39PM +0530, Sukrit Bhatnagar wrote:
> By making use of GNU C's cleanup attribute handled by the
> VIR_AUTOFREE macro for declaring scalar variables, majority
> of the VIR_FREE calls can be dropped, which in turn leads to
> getting rid of most of our cleanup sections.
>
> Signed-off-by: Sukrit Bhatnagar 
> ---
>  src/util/virnuma.c | 79 
> +-
>  1 file changed, 31 insertions(+), 48 deletions(-)
>
> diff --git a/src/util/virnuma.c b/src/util/virnuma.c
> index 784db0a..841c7cb 100644
> --- a/src/util/virnuma.c
> +++ b/src/util/virnuma.c
> @@ -252,8 +252,8 @@ int
>  virNumaGetNodeCPUs(int node,
> virBitmapPtr *cpus)
>  {
> -unsigned long *mask = NULL;
> -unsigned long *allonesmask = NULL;
> +VIR_AUTOFREE(unsigned long *) mask = NULL;
> +VIR_AUTOFREE(unsigned long *) allonesmask = NULL;
>  virBitmapPtr cpumap = NULL;
>  int ncpus = 0;
>  int max_n_cpus = virNumaGetMaxCPUs();
> @@ -300,8 +300,6 @@ virNumaGetNodeCPUs(int node,
>  ret = ncpus;
>
>   cleanup:
> -VIR_FREE(mask);
> -VIR_FREE(allonesmask);
>  virBitmapFree(cpumap);
>
>  return ret;
> @@ -566,52 +564,47 @@ virNumaGetHugePageInfo(int node,
> unsigned long long *page_avail,
> unsigned long long *page_free)
>  {
> -int ret = -1;
> -char *path = NULL;
> -char *buf = NULL;
>  char *end;
>
>  if (page_avail) {
> +VIR_AUTOFREE(char *) path = NULL;
> +VIR_AUTOFREE(char *) buf = NULL;

I don't believe this VIR_AUTOFREE duplication is necessary, just declare it at
the function level...

>  if (virNumaGetHugePageInfoPath(, node,
> page_size, "nr_hugepages") < 0)
> -goto cleanup;
> +return -1;
>
>  if (virFileReadAll(path, 1024, ) < 0)
> -goto cleanup;
> +return -1;
>
>  if (virStrToLong_ull(buf, , 10, page_avail) < 0 ||
>  *end != '\n') {
>  virReportError(VIR_ERR_INTERNAL_ERROR,
> _("unable to parse: %s"),
> buf);
> -goto cleanup;
> +return -1;
>  }
> -VIR_FREE(buf);
> -VIR_FREE(path);

... and leave ^these two in place.

>  }
>
>  if (page_free) {
> +VIR_AUTOFREE(char *) path = NULL;
> +VIR_AUTOFREE(char *) buf = NULL;
>  if (virNumaGetHugePageInfoPath(, node,
> page_size, "free_hugepages") < 0)
> -goto cleanup;
> +return -1;
>
>  if (virFileReadAll(path, 1024, ) < 0)
> -goto cleanup;
> +return -1;
>
>  if (virStrToLong_ull(buf, , 10, page_free) < 0 ||
>  *end != '\n') {
>  virReportError(VIR_ERR_INTERNAL_ERROR,
> _("unable to parse: %s"),
> buf);
> -goto cleanup;
> +return -1;
>  }
>  }

I'll fix those locally and merge.
Reviewed-by: Erik Skultety 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v1 24/32] util: numa: use VIR_AUTOFREE instead of VIR_FREE for scalar types

2018-07-28 Thread Sukrit Bhatnagar 
By making use of GNU C's cleanup attribute handled by the
VIR_AUTOFREE macro for declaring scalar variables, majority
of the VIR_FREE calls can be dropped, which in turn leads to
getting rid of most of our cleanup sections.

Signed-off-by: Sukrit Bhatnagar 
---
 src/util/virnuma.c | 79 +-
 1 file changed, 31 insertions(+), 48 deletions(-)

diff --git a/src/util/virnuma.c b/src/util/virnuma.c
index 784db0a..841c7cb 100644
--- a/src/util/virnuma.c
+++ b/src/util/virnuma.c
@@ -252,8 +252,8 @@ int
 virNumaGetNodeCPUs(int node,
virBitmapPtr *cpus)
 {
-unsigned long *mask = NULL;
-unsigned long *allonesmask = NULL;
+VIR_AUTOFREE(unsigned long *) mask = NULL;
+VIR_AUTOFREE(unsigned long *) allonesmask = NULL;
 virBitmapPtr cpumap = NULL;
 int ncpus = 0;
 int max_n_cpus = virNumaGetMaxCPUs();
@@ -300,8 +300,6 @@ virNumaGetNodeCPUs(int node,
 ret = ncpus;
 
  cleanup:
-VIR_FREE(mask);
-VIR_FREE(allonesmask);
 virBitmapFree(cpumap);
 
 return ret;
@@ -566,52 +564,47 @@ virNumaGetHugePageInfo(int node,
unsigned long long *page_avail,
unsigned long long *page_free)
 {
-int ret = -1;
-char *path = NULL;
-char *buf = NULL;
 char *end;
 
 if (page_avail) {
+VIR_AUTOFREE(char *) path = NULL;
+VIR_AUTOFREE(char *) buf = NULL;
 if (virNumaGetHugePageInfoPath(, node,
page_size, "nr_hugepages") < 0)
-goto cleanup;
+return -1;
 
 if (virFileReadAll(path, 1024, ) < 0)
-goto cleanup;
+return -1;
 
 if (virStrToLong_ull(buf, , 10, page_avail) < 0 ||
 *end != '\n') {
 virReportError(VIR_ERR_INTERNAL_ERROR,
_("unable to parse: %s"),
buf);
-goto cleanup;
+return -1;
 }
-VIR_FREE(buf);
-VIR_FREE(path);
 }
 
 if (page_free) {
+VIR_AUTOFREE(char *) path = NULL;
+VIR_AUTOFREE(char *) buf = NULL;
 if (virNumaGetHugePageInfoPath(, node,
page_size, "free_hugepages") < 0)
-goto cleanup;
+return -1;
 
 if (virFileReadAll(path, 1024, ) < 0)
-goto cleanup;
+return -1;
 
 if (virStrToLong_ull(buf, , 10, page_free) < 0 ||
 *end != '\n') {
 virReportError(VIR_ERR_INTERNAL_ERROR,
_("unable to parse: %s"),
buf);
-goto cleanup;
+return -1;
 }
 }
 
-ret = 0;
- cleanup:
-VIR_FREE(buf);
-VIR_FREE(path);
-return ret;
+return 0;
 }
 
 /**
@@ -714,13 +707,13 @@ virNumaGetPages(int node,
 size_t *npages)
 {
 int ret = -1;
-char *path = NULL;
+VIR_AUTOFREE(char *) path = NULL;
+VIR_AUTOFREE(unsigned int *) tmp_size = NULL;
+VIR_AUTOFREE(unsigned long long *) tmp_avail = NULL;
+VIR_AUTOFREE(unsigned long long *) tmp_free = NULL;
 DIR *dir = NULL;
 int direrr = 0;
 struct dirent *entry;
-unsigned int *tmp_size = NULL;
-unsigned long long *tmp_avail = NULL;
-unsigned long long *tmp_free = NULL;
 unsigned int ntmp = 0;
 size_t i;
 bool exchange;
@@ -828,11 +821,7 @@ virNumaGetPages(int node,
 *npages = ntmp;
 ret = 0;
  cleanup:
-VIR_FREE(tmp_free);
-VIR_FREE(tmp_avail);
-VIR_FREE(tmp_size);
 VIR_DIR_CLOSE(dir);
-VIR_FREE(path);
 return ret;
 }
 
@@ -843,8 +832,8 @@ virNumaSetPagePoolSize(int node,
unsigned long long page_count,
bool add)
 {
-int ret = -1;
-char *nr_path = NULL, *nr_buf =  NULL;
+VIR_AUTOFREE(char *) nr_path = NULL;
+VIR_AUTOFREE(char *) nr_buf =  NULL;
 char *end;
 unsigned long long nr_count;
 
@@ -853,37 +842,35 @@ virNumaSetPagePoolSize(int node,
  * differently to huge pages. */
 virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
_("system pages pool can't be modified"));
-goto cleanup;
+return -1;
 }
 
 if (virNumaGetHugePageInfoPath(_path, node, page_size, "nr_hugepages") 
< 0)
-goto cleanup;
+return -1;
 
 /* Firstly check, if there's anything for us to do */
 if (virFileReadAll(nr_path, 1024, _buf) < 0)
-goto cleanup;
+return -1;
 
 if (virStrToLong_ull(nr_buf, , 10, _count) < 0 ||
 *end != '\n') {
 virReportError(VIR_ERR_OPERATION_FAILED,
_("invalid number '%s' in '%s'"),
nr_buf, nr_path);
-goto cleanup;
+return -1;
 }
 
 if (add) {
 if (!page_count) {
 VIR_DEBUG("Nothing left to do: add = true page_count = 0");
-ret = 0;
-