[libvirt] [PATCH v1 18/21] util: identity: use VIR_AUTOFREE instead of VIR_FREE for scalar types

Erik Skultety eskultet at redhat.com
Mon Jun 25 14:20:21 UTC 2018


On Fri, Jun 08, 2018 at 01:04:40AM +0530, Sukrit Bhatnagar wrote:
> By making use of the GCC's __attribute__((cleanup)) handled by VIR_AUTOFREE
> macro, majority of the VIR_FREE calls can be dropped, which in turn leads to
> getting rid of most of our cleanup sections.

You're missing the SoB line here. Make sure to add that to comply with the
Developer Certificate of Origin, for more info, see
https://libvirt.org/governance.html#contributors

> ---
>  src/util/viridentity.c | 54 ++++++++++++++++++++------------------------------
>  1 file changed, 22 insertions(+), 32 deletions(-)
>
> diff --git a/src/util/viridentity.c b/src/util/viridentity.c
> index 2f4307b..2060dd7 100644
> --- a/src/util/viridentity.c
> +++ b/src/util/viridentity.c
> @@ -133,8 +133,8 @@ int virIdentitySetCurrent(virIdentityPtr ident)
>   */
>  virIdentityPtr virIdentityGetSystem(void)
>  {
> -    char *username = NULL;
> -    char *groupname = NULL;
> +    VIR_AUTOFREE(char *) username = NULL;
> +    VIR_AUTOFREE(char *) groupname = NULL;
>      unsigned long long startTime;
>      virIdentityPtr ret = NULL;
>  #if WITH_SELINUX
> @@ -154,14 +154,14 @@ virIdentityPtr virIdentityGetSystem(void)
>          goto error;
>
>      if (!(username = virGetUserName(geteuid())))
> -        goto cleanup;
> +        return ret;
>      if (virIdentitySetUNIXUserName(ret, username) < 0)
>          goto error;
>      if (virIdentitySetUNIXUserID(ret, getuid()) < 0)
>          goto error;
>
>      if (!(groupname = virGetGroupName(getegid())))
> -        goto cleanup;
> +        return ret;
>      if (virIdentitySetUNIXGroupName(ret, groupname) < 0)
>          goto error;
>      if (virIdentitySetUNIXGroupID(ret, getgid()) < 0)
> @@ -172,7 +172,7 @@ virIdentityPtr virIdentityGetSystem(void)
>          if (getcon(&con) < 0) {
>              virReportSystemError(errno, "%s",
>                                   _("Unable to lookup SELinux process context"));
> -            goto cleanup;
> +            return ret;
>          }
>          if (virIdentitySetSELinuxContext(ret, con) < 0) {
>              freecon(con);
> @@ -182,15 +182,9 @@ virIdentityPtr virIdentityGetSystem(void)
>      }
>  #endif
>
> - cleanup:
> -    VIR_FREE(username);
> -    VIR_FREE(groupname);
> -    return ret;
> -
>   error:
>      virObjectUnref(ret);
> -    ret = NULL;
> -    goto cleanup;
> +    return NULL;

you're returning NULL on success too, which causes the test suite to fail, make
sure that you run make check -j X && make syntax-check -j X after every patch
in the series.

X - number of logical CPUs + 1

I briefly went through the other VIR_AUTOFREE patches and didn't spot any
problems, I'll have a better look once you post another version of this series
due to the points I raised.

Regards,
Erik




More information about the libvir-list mailing list