Re: [PATCH v4 12/23] libxl: add libxl_device_pci_assignable_list_free()...

[Oleksandr Andrushchenko <Oleksandr_Andrushchenko@xxxxxxxx>]

Hi, Paul!

On 11/24/20 10:01 AM, Paul Durrant wrote:
> From: Paul Durrant <pdurrant@xxxxxxxxxx>
>
> ... to be used by callers of libxl_device_pci_assignable_list().
>
> Currently there is no API for callers of libxl_device_pci_assignable_list()
> to free the list. The xl function pciassignable_list() calls
> libxl_device_pci_dispose() on each element of the returned list, but
> libxl_pci_assignable() in libxl_pci.c does not. Neither does the 
> implementation
> of libxl_device_pci_assignable_list() call libxl_device_pci_init().
>
> This patch adds the new API function, makes sure it is used everywhere and
> also modifies libxl_device_pci_assignable_list() to initialize list
> entries rather than just zeroing them.
>
> Signed-off-by: Paul Durrant <pdurrant@xxxxxxxxxx>
Reviewed-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@xxxxxxxx>

Thank you,

Oleksandr

> ---
> Cc: Ian Jackson <iwj@xxxxxxxxxxxxxx>
> Cc: Wei Liu <wl@xxxxxxx>
> Cc: Christian Lindig <christian.lindig@xxxxxxxxxx>
> Cc: David Scott <dave@xxxxxxxxxx>
> Cc: Anthony PERARD <anthony.perard@xxxxxxxxxx>
> ---
>   tools/include/libxl.h                |  7 +++++++
>   tools/libs/light/libxl_pci.c         | 14 ++++++++++++--
>   tools/ocaml/libs/xl/xenlight_stubs.c |  3 +--
>   tools/xl/xl_pci.c                    |  3 +--
>   4 files changed, 21 insertions(+), 6 deletions(-)
>
> diff --git a/tools/include/libxl.h b/tools/include/libxl.h
> index ee52d3cf7e..8225809d94 100644
> --- a/tools/include/libxl.h
> +++ b/tools/include/libxl.h
> @@ -458,6 +458,12 @@
>   #define LIBXL_HAVE_DEVICE_PCI_LIST_FREE 1
>   
>   /*
> + * LIBXL_HAVE_DEVICE_PCI_ASSIGNABLE_LIST_FREE indicates that the
> + * libxl_device_pci_assignable_list_free() function is defined.
> + */
> +#define LIBXL_HAVE_DEVICE_PCI_ASSIGNABLE_LIST_FREE 1
> +
> +/*
>    * libxl ABI compatibility
>    *
>    * The only guarantee which libxl makes regarding ABI compatibility
> @@ -2369,6 +2375,7 @@ int libxl_device_events_handler(libxl_ctx *ctx,
>   int libxl_device_pci_assignable_add(libxl_ctx *ctx, libxl_device_pci *pci, 
> int rebind);
>   int libxl_device_pci_assignable_remove(libxl_ctx *ctx, libxl_device_pci 
> *pci, int rebind);
>   libxl_device_pci *libxl_device_pci_assignable_list(libxl_ctx *ctx, int 
> *num);
> +void libxl_device_pci_assignable_list_free(libxl_device_pci *list, int num);
>   
>   /* CPUID handling */
>   int libxl_cpuid_parse_config(libxl_cpuid_policy_list *cpuid, const char* 
> str);
> diff --git a/tools/libs/light/libxl_pci.c b/tools/libs/light/libxl_pci.c
> index 0f41939d1f..5a3352c2ec 100644
> --- a/tools/libs/light/libxl_pci.c
> +++ b/tools/libs/light/libxl_pci.c
> @@ -457,7 +457,7 @@ libxl_device_pci 
> *libxl_device_pci_assignable_list(libxl_ctx *ctx, int *num)
>           pcis = new;
>           new = pcis + *num;
>   
> -        memset(new, 0, sizeof(*new));
> +        libxl_device_pci_init(new);
>           pci_struct_fill(new, dom, bus, dev, func, 0);
>   
>           if (pci_info_xs_read(gc, new, "domid")) /* already assigned */
> @@ -472,6 +472,16 @@ out:
>       return pcis;
>   }
>   
> +void libxl_device_pci_assignable_list_free(libxl_device_pci *list, int num)
> +{
> +    int i;
> +
> +    for (i = 0; i < num; i++)
> +        libxl_device_pci_dispose(&list[i]);
> +
> +    free(list);
> +}
> +
>   /* Unbind device from its current driver, if any.  If driver_path is 
> non-NULL,
>    * store the path to the original driver in it. */
>   static int sysfs_dev_unbind(libxl__gc *gc, libxl_device_pci *pci,
> @@ -1490,7 +1500,7 @@ static int libxl_pci_assignable(libxl_ctx *ctx, 
> libxl_device_pci *pci)
>               pcis[i].func == pci->func)
>               break;
>       }
> -    free(pcis);
> +    libxl_device_pci_assignable_list_free(pcis, num);
>       return i != num;
>   }
>   
> diff --git a/tools/ocaml/libs/xl/xenlight_stubs.c 
> b/tools/ocaml/libs/xl/xenlight_stubs.c
> index 1181971da4..352a00134d 100644
> --- a/tools/ocaml/libs/xl/xenlight_stubs.c
> +++ b/tools/ocaml/libs/xl/xenlight_stubs.c
> @@ -894,9 +894,8 @@ value stub_xl_device_pci_assignable_list(value ctx)
>               Field(list, 1) = temp;
>               temp = list;
>               Store_field(list, 0, Val_device_pci(&c_list[i]));
> -             libxl_device_pci_dispose(&c_list[i]);
>       }
> -     free(c_list);
> +     libxl_device_pci_assignable_list_free(c_list, nb);
>   
>       CAMLreturn(list);
>   }
> diff --git a/tools/xl/xl_pci.c b/tools/xl/xl_pci.c
> index 7c0f102ac7..f71498cbb5 100644
> --- a/tools/xl/xl_pci.c
> +++ b/tools/xl/xl_pci.c
> @@ -164,9 +164,8 @@ static void pciassignable_list(void)
>       for (i = 0; i < num; i++) {
>           printf("%04x:%02x:%02x.%01x\n",
>                  pcis[i].domain, pcis[i].bus, pcis[i].dev, pcis[i].func);
> -        libxl_device_pci_dispose(&pcis[i]);
>       }
> -    free(pcis);
> +    libxl_device_pci_assignable_list_free(pcis, num);
>   }
>   
>   int main_pciassignable_list(int argc, char **argv)