Re: [PATCH] libxl: Use g_autofree for char* where easily possible

[Ján Tomko <jtomko@xxxxxxxxxx>]

On a Tuesday in 2021, Jim Fehlig wrote:
> All of these strings are allocated once, freed once, and are never
> returned out of the function where they are declared.
>
> Signed-off-by: Jim Fehlig <jfehlig@xxxxxxxx>
> ---
> src/libxl/libxl_capabilities.c |  3 +--
> src/libxl/libxl_domain.c       | 17 +++++------------
> src/libxl/libxl_driver.c       | 32 +++++++++++---------------------
> src/libxl/libxl_logger.c       | 21 ++++++---------------
> src/libxl/libxl_migration.c    | 15 +++++----------
> src/libxl/xen_common.c         | 17 ++++++-----------
> src/libxl/xen_xl.c             |  6 ++----
> 7 files changed, 36 insertions(+), 75 deletions(-)
>
> @@ -1262,8 +1258,8 @@ libxlDomainStart(libxlDriverPrivatePtr driver,
>     libxlSavefileHeader hdr;
>     int ret = -1;
>     uint32_t domid = 0;
> -    char *dom_xml = NULL;
> -    char *managed_save_path = NULL;
> +    g_autofree char *dom_xml = NULL;
> +    g_autofree char *managed_save_path = NULL;
>     int managed_save_fd = -1;
>     libxlDomainObjPrivatePtr priv = vm->privateData;
>     g_autoptr(libxlDriverConfig) cfg = libxlDriverConfigGet(driver);
> @@ -1271,7 +1267,7 @@ libxlDomainStart(libxlDriverPrivatePtr driver,
>     libxl_asyncprogress_how aop_console_how;
>     libxl_domain_restore_params params;
>     unsigned int hostdev_flags = VIR_HOSTDEV_SP_PCI;
> -    char *config_json = NULL;
> +    g_autofree char *config_json = NULL;
>
> #ifdef LIBXL_HAVE_PVUSB
>     hostdev_flags |= VIR_HOSTDEV_SP_USB;

There is one more free that can be removed in the if (restore_fd < 0) block:

             vm->hasManagedSave = false;
         }
         VIR_FREE(managed_save_path);
     }

     if (virDomainObjSetDefTransient(driver->xmlopt, vm, NULL) < 0)
         goto cleanup;


> @@ -1513,9 +1509,6 @@ libxlDomainStart(libxlDriverPrivatePtr driver,
>
>  cleanup:
>     libxl_domain_config_dispose(&d_config);
> -    VIR_FREE(config_json);
> -    VIR_FREE(dom_xml);
> -    VIR_FREE(managed_save_path);
>     virDomainDefFree(def);
>     VIR_FORCE_CLOSE(managed_save_fd);
>     return ret;

[...]

> @@ -4882,7 +4876,7 @@ libxlDomainGetNumaParameters(virDomainPtr dom,
>     virDomainObjPtr vm;
>     libxl_bitmap nodemap;
>     virBitmapPtr nodes = NULL;
> -    char *nodeset = NULL;
> +    g_autofree char *nodeset = NULL;

This variable is used inside the loop, please move its declaration
there (even though it's only used once).

>     int rc, ret = -1;
>     size_t i, j;
>
> @@ -4981,7 +4975,6 @@ libxlDomainGetNumaParameters(virDomainPtr dom,
>     ret = 0;
>
>  cleanup:
> -    VIR_FREE(nodeset);
>     virBitmapFree(nodes);
>     libxl_bitmap_dispose(&nodemap);
>     virDomainObjEndAPI(&vm);

With the logger changes from your reply:

Reviewed-by: Ján Tomko <jtomko@xxxxxxxxxx>

Jano

Attachment: signature.asc
Description: PGP signature