Re: [PATCH RFC V2 3/5] libxl: add pvusb API

["Jürgen Groß" <jgross@suse.com>]

Just found other issues:

On 01/19/2015 09:28 AM, Chunyan Liu wrote:
> Add pvusb APIs, including:
>   - attach/detach (create/destroy) virtual usb controller.
>   - attach/detach usb device
>   - list assignable usb devices in host
>   - some other helper functions
>
> Signed-off-by: Chunyan Liu <cyliu@suse.com>
> Signed-off-by: Simon Cao <caobosimon@gmail.com>
> ---
>   tools/libxl/Makefile         |    2 +-
>   tools/libxl/libxl.c          |    2 +
>   tools/libxl/libxl.h          |   58 ++
>   tools/libxl/libxl_internal.h |    6 +
>   tools/libxl/libxl_usb.c      | 1277 ++++++++++++++++++++++++++++++++++++++++++
>   tools/libxl/libxlu_cfg_y.c   |  464 ++++++++-------
>   tools/libxl/libxlu_cfg_y.h   |   38 +-
>   7 files changed, 1623 insertions(+), 224 deletions(-)
>   create mode 100644 tools/libxl/libxl_usb.c
>
...
> +/*Get usb device information */
> +static int get_usb_devnum (libxl__gc *gc, const char *intf, char *buf)
> +{
> +    char *path;
> +    int rc = 0;
> +    FILE *fd;
> +
> +    path = libxl__sprintf(gc, "cat "SYSFS_USB_DEVS_PATH"/%s/devnum", intf);
> +    fd = popen(path, "r");
> +    if (fgets(buf, 512, fd) == NULL || ferror(fd))
> +        rc = -1;
> +    pclose(fd);
> +
> +    return rc;
> +}
> +
> +static int get_usb_busnum(libxl__gc *gc, const char *intf, char *buf)
> +{
> +    char *path;
> +    int rc = 0;
> +    FILE *fd;
> +
> +    path = libxl__sprintf(gc, "cat "SYSFS_USB_DEVS_PATH"/%s/busnum", intf);
> +    fd = popen(path, "r");
> +    if (fgets(buf, 512, fd) == NULL || ferror(fd))
> +        rc = -1;
> +    pclose(fd);
> +
> +    return rc;
> +}
> +
> +static int get_usb_idVendor(libxl__gc *gc, const char *intf, char *buf)
> +{
> +    char *path;
> +    int rc = 0;
> +    FILE *fd;
> +
> +    path = libxl__sprintf(gc, "cat "SYSFS_USB_DEVS_PATH"/%s/idVendor", intf);
> +    fd = popen(path, "r");
> +    if (fgets(buf, 512, fd) == NULL || ferror(fd))
> +        rc = -1;
> +    pclose(fd);
> +
> +    return rc;
> +}
> +
> +static int get_usb_idProduct(libxl__gc *gc, const char *intf, char *buf)
> +{
> +    char *path;
> +    int rc = 0;
> +    FILE *fd;
> +
> +    path = libxl__sprintf(gc, "cat "SYSFS_USB_DEVS_PATH"/%s/idProduct", intf);
> +    fd = popen(path, "r");
> +    if (fgets(buf, 512, fd) == NULL || ferror(fd))
> +        rc = -1;
> +    pclose(fd);
> +
> +    return rc;
> +}
> +
> +static int get_usb_manufacturer(libxl__gc *gc, const char *intf, char *buf)
> +{
> +    char *path;
> +    int rc = 0;
> +    FILE *fd;
> +
> +    path = libxl__sprintf(gc, "cat "SYSFS_USB_DEVS_PATH"/%s/manufacturer", intf);

File does not exist in newer kernels (checked 3.16 and 3.19).

> +    fd = popen(path, "r");
> +    if (fgets(buf, 512, fd) == NULL || ferror(fd))
> +        rc = -1;

Another reason not to use popen(): file doesn't exist, but rc = 0.
And buf contains garbage.

> +    pclose(fd);
> +
> +    return rc;
> +}
> +
> +static int get_usb_product(libxl__gc *gc, const char *intf, char *buf)
> +{
> +    char *path;
> +    int rc = 0;
> +    FILE *fd;
> +
> +    path = libxl__sprintf(gc, "cat "SYSFS_USB_DEVS_PATH"/%s/product", intf);

File does not exist in newer kernels (checked 3.16 and 3.19).

> +    fd = popen(path, "r");
> +    if (fgets(buf, 512, fd) == NULL || ferror(fd))
> +        rc = -1;
> +    pclose(fd);
> +
> +    return rc;
> +}
> +
> +int libxl_device_usb_getinfo(libxl_ctx *ctx, char *intf, libxl_usbinfo *usbinfo)
> +{
> +    GC_INIT(ctx);
> +    char buf[512];
> +
> +    if (!get_usb_devnum(gc, intf, buf) )
> +        usbinfo->devnum = atoi(buf);
> +
> +    if ( !get_usb_busnum(gc, intf, buf))
> +        usbinfo->bus = atoi(buf);
> +
> +    if (!get_usb_idVendor(gc, intf, buf) )
> +        usbinfo->idVendor = atoi(buf);

atoi is wrong. idVendor in sysfs is a hex-string.

> +
> +    if (!get_usb_idProduct(gc, intf, buf) )
> +        usbinfo->idProduct  = atoi(buf);

again hex-string.

> +
> +    if (!get_usb_manufacturer(gc, intf, buf) )
> +        usbinfo->manuf = strdup(buf);
> +
> +    if (!get_usb_product(gc, intf, buf) )
> +        usbinfo->prod = strdup(buf);
> +
> +    GC_FREE;
> +    return 0;
> +}