Re: [PATCH 02/30] util: convert pointers to use g_autofree

[Ján Tomko <jtomko@xxxxxxxxxx>]

util: vhba: convert pointers to use g_autofree

On a Monday in 2020, Ryan Gahagan wrote:
> From: Barrett Schonefeld <bschoney@xxxxxxxxxx>

> - src/util/virvhba.c

No need to mention the file here.

> Signed-off-by: Barrett Schonefeld <bschoney@xxxxxxxxxx>
> ---
> src/util/virvhba.c | 35 ++++++++++++-----------------------
> 1 file changed, 12 insertions(+), 23 deletions(-)
>
> diff --git a/src/util/virvhba.c b/src/util/virvhba.c
> index a4e88024d1..a80145b8fd 100644
> --- a/src/util/virvhba.c
> +++ b/src/util/virvhba.c

> @@ -160,8 +155,8 @@ virVHBAFindVportHost(const char *sysfs_prefix)
>     const char *prefix = sysfs_prefix ? sysfs_prefix : SYSFS_FC_HOST_PATH;
>     g_autoptr(DIR) dir = NULL;
>     struct dirent *entry = NULL;
> -    char *max_vports = NULL;
> -    char *vports = NULL;
> +    g_autofree char *max_vports = NULL;
> +    g_autofree char *vports = NULL;
>     char *state = NULL;
>     char *ret = NULL;
>
> @@ -220,8 +215,6 @@ virVHBAFindVportHost(const char *sysfs_prefix)
>     }
>
>  cleanup:
> -    VIR_FREE(max_vports);
> -    VIR_FREE(vports);

These two are also freed within the while loop above.
We try to avoid mixing autofree with calls to VIR_FREE/g_free,
to make things simple.

Here, you can simply move the declaration of the two variables at the
top of the loop, since they're freed onece per loop iteration.

Same for the 'state' variable.

>     return ret;
> }
>
> @@ -241,7 +234,8 @@ virVHBAManageVport(const int parent_host,
>                    int operation)
> {
>     int ret = -1;
> -    char *operation_path = NULL, *vport_name = NULL;
> +    g_autofree char *operation_path = NULL;
> +    g_autofree char *vport_name = NULL;
>     const char *operation_file = NULL;
>
>     switch (operation) {
> @@ -291,8 +285,6 @@ virVHBAManageVport(const int parent_host,
>                              vport_name, operation_path);
>
>  cleanup:
> -    VIR_FREE(vport_name);
> -    VIR_FREE(operation_path);

operation_path is also freed in the middle of the function.
But I cannot think of an elegant solution to that one.

>     return ret;
> }
>
> @@ -315,8 +307,8 @@ vhbaReadCompareWWN(const char *prefix,
>                    const char *f_name,
>                    const char *wwn)
> {
> -    char *path;
> -    char *buf = NULL;
> +    g_autofree char *path = NULL;
> +    g_autofree char *buf = NULL;
>     char *p;
>     int ret = -1;
>
> @@ -343,8 +335,6 @@ vhbaReadCompareWWN(const char *prefix,
>         ret = 1;
>
>  cleanup:
> -    VIR_FREE(path);
> -    VIR_FREE(buf);
>
>     return ret;
> }
> @@ -407,7 +397,7 @@ virVHBAGetHostByFabricWWN(const char *sysfs_prefix,
>     const char *prefix = sysfs_prefix ? sysfs_prefix : SYSFS_FC_HOST_PATH;
>     struct dirent *entry = NULL;
>     g_autoptr(DIR) dir = NULL;
> -    char *vport_create_path = NULL;
> +    g_autofree char *vport_create_path = NULL;

This one should be also moved into the while loop.

Jano

>     char *ret = NULL;
>
>     if (virDirOpen(&dir, prefix) < 0)
> @@ -438,7 +428,6 @@ virVHBAGetHostByFabricWWN(const char *sysfs_prefix,
>     }
>
>  cleanup:
> -    VIR_FREE(vport_create_path);
>     return ret;
> }
>
> --
> 2.29.0

Attachment: signature.asc
Description: PGP signature