Re: [Xen-devel] [PATCH v4 02/13] libxl: add generic functions to get and free device list

[Oleksandr Grytsov <al1img@xxxxxxxxx>]

On Tue, Sep 5, 2017 at 2:51 PM, Wei Liu <wei.liu2@xxxxxxxxxx> wrote:
> On Tue, Jul 18, 2017 at 05:25:19PM +0300, Oleksandr Grytsov wrote:
>> From: Oleksandr Grytsov <oleksandr_grytsov@xxxxxxxx>
>>
>> Add libxl__device_list and libxl__device_list_free
>> functions to handle device list using the device
>> framework.
>>
>> Signed-off-by: Oleksandr Grytsov <oleksandr_grytsov@xxxxxxxx>
>> ---
>>  tools/libxl/libxl_device.c   | 66 
>> ++++++++++++++++++++++++++++++++++++++++++++
>>  tools/libxl/libxl_internal.h |  8 ++++++
>>  2 files changed, 74 insertions(+)
>>
>> diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
>> index 07165f0..f1d4848 100644
>> --- a/tools/libxl/libxl_device.c
>> +++ b/tools/libxl/libxl_device.c
>> @@ -1991,6 +1991,72 @@ out:
>>      return rc;
>>  }
>>
>> +void *libxl__device_list(libxl__gc *gc, const struct libxl_device_type *dt,
>> +                         uint32_t domid, const char* name, int *num)
>> +{
>> +    void *r = NULL;
>> +    void *list = NULL;
>> +    void *item = NULL;
>> +    char *libxl_path;
>> +    char **dir = NULL;
>> +    unsigned int ndirs = 0;
>> +    int rc;
>> +
>> +    *num = 0;
>> +
>> +    libxl_path = GCSPRINTF("%s/device/%s",
>> +                           libxl__xs_libxl_path(gc, domid), name);
>> +
>> +    dir = libxl__xs_directory(gc, XBT_NULL, libxl_path, &ndirs);
>> +
>> +    if (dir && ndirs) {
>> +        list = libxl__malloc(NOGC, dt->dev_elem_size * ndirs);
>> +        void *end = (uint8_t *)list + ndirs * dt->dev_elem_size;
>> +        item = list;
>> +
>> +        while (item < end) {
>> +            dt->init(item);
>> +
>> +            if (dt->from_xenstore) {
>> +                char* device_libxl_path = GCSPRINTF("%s/%s", libxl_path, 
>> *dir);
>> +                rc = dt->from_xenstore(gc, device_libxl_path, atoi(*dir), 
>> item);
>> +                if (rc) goto out;
>> +            }
>> +
>> +            item = (uint8_t*)item + dt->dev_elem_size;
>
> Space before *.
>
>> +            ++dir;
>> +        }
>> +    }
>> +
>> +    *num = ndirs;
>> +    r = list;
>> +    list = NULL;
>> +
>> +out:
>> +
>> +    if (list) {
>> +        *num = 0;
>> +        while(item >= list) {
>
> Space after while, but ...
>
>> +            dt->dispose(item);
>> +            item = (uint8_t*)item - dt->dev_elem_size;
>> +        }
>> +        free(list);
>
> You should be able to use libxl__device_list_free here.

Good catch. I will use list_free here. This requires slight changes in
above loop:
i need to count initialized elements in order to pass it to list_free.

>
>> +    }
>> +
>> +    return r;
>> +}
>> +
>> +void libxl__device_list_free(const struct libxl_device_type *dt,
>> +                             void *list, int num)
>> +{
>> +    int i;
>> +
>> +    for (i = 0; i < num; i++)
>> +        dt->dispose((uint8_t*)list + i * dt->dev_elem_size);
>> +
>> +    free(list);
>> +}
>> +
>>  /*
>>   * Local variables:
>>   * mode: C
>> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
>> index 075dfe3..271ac89 100644
>> --- a/tools/libxl/libxl_internal.h
>> +++ b/tools/libxl/libxl_internal.h
>> @@ -3506,6 +3506,7 @@ struct libxl_device_type {
>>      int (*dm_needed)(void *, unsigned);
>>      void (*update_config)(libxl__gc *, void *, void *);
>>      int (*update_devid)(libxl__gc *, uint32_t, void *);
>> +    int (*from_xenstore)(libxl__gc *, const char *, libxl_devid, void *);
>>      int (*set_xenstore_config)(libxl__gc *, uint32_t, void *, flexarray_t *,
>>                                 flexarray_t *, flexarray_t *);
>>  };
>> @@ -4386,6 +4387,13 @@ void libxl__device_add_async(libxl__egc *egc, 
>> uint32_t domid,
>>  int libxl__device_add(libxl__gc *gc, uint32_t domid,
>>                        const struct libxl_device_type *dt, void *type);
>>
>> +/* Caller is responsible for freeing the memory by calling
>> + * libxl__device_list_free
>> + */
>> +void* libxl__device_list(libxl__gc *gc, const struct libxl_device_type *dt,
>> +                         uint32_t domid, const char* name, int *num);
>> +void libxl__device_list_free(const struct libxl_device_type *dt,
>> +                             void *list, int num);
>>  #endif
>>
>>  /*
>> --
>> 2.7.4
>>

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel