[Spice-devel] [spice-gtk 09/13] win-usb-dev: do not allocate memory for hub devices

[Yuri Benditovich]

On Tue, Mar 12, 2019 at 4:11 PM Christophe Fergeau <cfergeau at redhat.com> wrote:
>
> On Sun, Mar 10, 2019 at 04:46:08PM +0200, Yuri Benditovich wrote:
> > When processing list of USB devices, avoid allocating memory
> > for devices which later will be skipped. Use existing libusb
> > struct first to check whether the device shall be excluded.
>
> I probably would reword the short log
> "win-usb-dev: do not allocate memory for hub devices"
> -> "win-usb-dev: skip hub devices early in g_udev_client_list_devices"
>
> > -static gboolean g_udev_skip_search(GUdevDevice *udev)
> > +static gboolean g_udev_skip_search(libusb_device *dev)
> >  {
> > -    GUdevDeviceInfo* udevinfo;
> >      gboolean skip;
> > +    uint8_t addr  = libusb_get_device_address(dev);
> > +    struct libusb_device_descriptor desc;
> >
> > -    g_return_val_if_fail(G_UDEV_DEVICE(udev), FALSE);
> > -
> > -    udevinfo = udev->priv->udevinfo;
> > -    g_return_val_if_fail(udevinfo != NULL, FALSE);
> > +    libusb_get_device_descriptor(dev, &desc);
>
> Do you know if these
> libusb_get_device_descriptor/libusb_get_device_address are going to be
> expensive compared to using cached values?

These libusb calls use cached values as well

>
> >
> > -    skip = ((udevinfo->addr == 0xff) ||  /* root hub (HCD) */
> > +    skip = ((addr == 0xff) ||  /* root hub (HCD) */
> >  #if defined(LIBUSBX_API_VERSION) && (LIBUSBX_API_VERSION >= 0x010000FF)
> > -            (udevinfo->addr == 1) || /* root hub addr for libusbx >= 1.0.13 */
> > +            (addr == 1) || /* root hub addr for libusbx >= 1.0.13 */
>
> Note for later: I'm not sure this still needs to be conditional as we
> depend on libusb 1.0.16 now.
>
> Looks good otherwise,
>
> Christophe
> -----BEGIN PGP SIGNATURE-----
>
> iQIzBAEBCAAdFiEElKn3VmH3emFoZJsjqdjCFCmsbIIFAlyHvhgACgkQqdjCFCms
> bIK/hw//bGKxBvjPvFhXL8qjOWG0xqRnZf1Qxoi6VquDEeQtZlWmT13ojGe4m2Si
> YRoXHF4/yx7Bq24It0igfHxppZonRGnHkPu4VVVc874I8d1rif7Vf1c/1sHeKiPQ
> 247G/Nrehevrilyn/mx/9C8whY0zedzmPIFATHHMXiFHK/5q5OOhdc3304UeWPWL
> mupgOf6opcX7Ncqnvjd/OSo0D1b0LXE4/+2dLFVdBEirkiTEdA9JYEC6zeiKlr1U
> KZGfPpUmd7XX18Gm27G8xqBQ4n18MqoZIhMt2P6QfJODz3TPXy4zkGik0DJnAnzR
> e+jkZPrStLYNwEohgIDkmkvStheMRAM+nzaJrTzUZh5PaVxBDmT9mVKvS/zC8/nV
> TISaeLBX+Nva1tdfoaXMI8q97wmycqGtpFT+mJKSLJO3wUSKDKlekkOzvh+/9E4Q
> jlLYPAYLrXPqtBlZ2LjCrMItG60IlnMAG2t9nDBjZcXWfnYqv+OyGb4dP8nvLUQ4
> kE21x7zgOwT+AAJk+qtHq+aJQzqDaMaNdbqJbBsNiEL0Ow/J+puecSkaPpqEwf9e
> afIDUwA1joeg9Z00rx29kwJDobgKSgAQqDRFakSxVJok8GHt+j64q9H5bvi6jG2Z
> liXCArJo8Kg9b0HyAp8ebFm3msnwLiGyoLU5wq0QsVe3naVNgcQ=
> =CBPi
> -----END PGP SIGNATURE-----