[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



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 <skrtbhtngr gmail com>
> ---
>  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(&path, node,
>                                         page_size, "nr_hugepages") < 0)
> -            goto cleanup;
> +            return -1;
>
>          if (virFileReadAll(path, 1024, &buf) < 0)
> -            goto cleanup;
> +            return -1;
>
>          if (virStrToLong_ull(buf, &end, 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(&path, node,
>                                         page_size, "free_hugepages") < 0)
> -            goto cleanup;
> +            return -1;
>
>          if (virFileReadAll(path, 1024, &buf) < 0)
> -            goto cleanup;
> +            return -1;
>
>          if (virStrToLong_ull(buf, &end, 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 <eskultet redhat com>


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]