Re: [Xen-devel] [PATCH v10 3/5] libxl: add pvusb API

[George Dunlap <George.Dunlap@xxxxxxxxxxxxx>]

On Mon, Dec 14, 2015 at 7:25 AM, Chun Yan Liu <cyliu@xxxxxxxx> wrote:
>
>
>>>> On 12/10/2015 at 08:08 PM, in message <56696B4B.7060801@xxxxxxxxxx>, George
> Dunlap <george.dunlap@xxxxxxxxxx> wrote:
>> On 10/12/15 12:05, George Dunlap wrote:
>> > From: Chunyan Liu <cyliu@xxxxxxxx>
>> >
>> > Add pvusb APIs, including:
>> >  - attach/detach (create/destroy) virtual usb controller.
>> >  - attach/detach usb device
>> >  - list usb controller and usb devices
>> >  - some other helper functions
>> >
>> > Signed-off-by: Chunyan Liu <cyliu@xxxxxxxx>
>> > Signed-off-by: Simon Cao <caobosimon@xxxxxxxxx>
>> > Signed-off-by: George Dunlap <george.dunlap@xxxxxxxxxx>
>>
>> Attached is a diff of v9 -> v10 for convenience.
>
> Thanks very much, George!
> I've applied your new patch and tested, there are a couple of changes needed 
> to
> get tests PASSED. A small extra patch is written on top of your new patch, as 
> in
> attachment, please have a look.

Thanks -- the changes in the patch look good.

>> > +static int usbdev_get_all_interfaces(libxl__gc *gc, const char *busid,
>> > +                                     char ***intfs, int *num)
>> > +{
>> > +    DIR *dir;
>> > +    char *buf;
>> > +    int rc;
>> > +
>> > +    *intfs = NULL;
>> > +    *num = 0;
>> > +
>> > +    buf = GCSPRINTF("%s:", busid);
>> > +
>> > +    dir = opendir(SYSFS_USB_DEV);
>> > +    if (!dir) {
>> > +        LOGE(ERROR, "opendir failed: '%s'", SYSFS_USB_DEV);
>> > +        return ERROR_FAIL;
>> > +    }
>> > +
>> > +    size_t need = offsetof(struct dirent, d_name) +
>> > +        pathconf(SYSFS_USB_DEV, _PC_NAME_MAX) + 1;
>> > +    struct dirent *de_buf = libxl__zalloc(gc, need);
>>
>> Is this thing with manually calculating the size of the structure really
>> necessary?  Could we not just declare "struct dirent de_buf" on the stack?
>
> Calculating in above way is to allocate enough space for d_name, whereas
> "struct dirent de_buf" won't allocate space for d_name (which is char *).
>
> Codes for calling read_dir_r are often done like above.

OK -- in that case, can you put the allocation of the structure into a
macro or helper function, fold in the patch you sent, and re-send this
series as v11?

Thanks!

 -George

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