From c32774c8a46aa04bf8f882ea552df8cb2d9f3a59 Mon Sep 17 00:00:00 2001
From: Victor Toso <victortoso@redhat.com>
Date: Thu, 22 Dec 2022 16:58:43 +0100
Subject: [PATCH 8/8] usbredirect: use the correct bus-device

This patch is a continuation from:
"usbredirect: allow multiple devices by vendor:product"

As we were using libusb_open_device_with_vid_pid(), if an user
requested that device on 2-3 was redirected, we would instead get the
vendor and product info of device on 2-3 and use that info to pick a
usb device. This is wrong when multiple devices that shared
vendor:product are plugged as libbusb_open_device_with_vid_pid()
always return the same (first) device.

This commit now stores bus-device info and uses that to pick the usb
device to redirect.

Related: https://gitlab.freedesktop.org/spice/usbredir/-/issues/29
Signed-off-by: Victor Toso <victortoso@redhat.com>
---
 tools/usbredirect.c | 63 ++++++++++++++++++++-------------------------
 1 file changed, 28 insertions(+), 35 deletions(-)

diff --git a/tools/usbredirect.c b/tools/usbredirect.c
index 0b04418..cad9d23 100644
--- a/tools/usbredirect.c
+++ b/tools/usbredirect.c
@@ -24,9 +24,14 @@
 
 typedef struct redirect {
     struct {
+        /* vendor:product */
         int vendor;
         int product;
+        /* bus-device */
+        int bus;
+        int device_number;
     } device;
+    bool by_bus;
     bool is_client;
     bool keepalive;
     bool watch_inout;
@@ -46,7 +51,7 @@ typedef struct redirect {
 static void create_watch(redirect *self);
 
 static bool
-parse_opt_device(const char *device, int *vendor, int *product)
+parse_opt_device(redirect *self, const char *device)
 {
     if (!device) {
         g_warning("No device to redirect. For testing only\n");
@@ -54,37 +59,15 @@ parse_opt_device(const char *device, int *vendor, int *product)
     }
 
     if (g_strrstr(device, "-") != NULL) {
-        /* Get vendor and product by bus and address number */
+        self->by_bus = true;
         char **usbid = g_strsplit(device, "-", 2);
         if (usbid == NULL || usbid[0] == NULL || usbid[1] == NULL || usbid[2] != NULL) {
             g_strfreev(usbid);
             return false;
         }
-        gint64 bus = g_ascii_strtoll(usbid[0], NULL, 10);
-        gint64 addr = g_ascii_strtoll(usbid[1], NULL, 10);
-
-        libusb_device **list = NULL;
-        ssize_t i, n;
-
-        n = libusb_get_device_list(NULL, &list);
-        for (i = 0; i < n; i++) {
-            if (libusb_get_bus_number(list[i]) == bus &&
-                    libusb_get_device_address(list[i]) == addr) {
-                break;
-            }
-        }
-
-        if (i == n) {
-            libusb_free_device_list(list, true);
-            return false;
-        }
-
-        struct libusb_device_descriptor desc;
-        libusb_get_device_descriptor(list[i], &desc);
-        *vendor = desc.idVendor;
-        *product = desc.idProduct;
-
-        libusb_free_device_list(list, true);
+        self->device.bus = g_ascii_strtoll(usbid[0], NULL, 10);
+        self->device.device_number = g_ascii_strtoll(usbid[1], NULL, 10);
+        g_strfreev(usbid);
         return true;
     }
 
@@ -94,12 +77,14 @@ parse_opt_device(const char *device, int *vendor, int *product)
         return false;
     }
 
-    *vendor = g_ascii_strtoll(usbid[0], NULL, 16);
-    *product = g_ascii_strtoll(usbid[1], NULL, 16);
+    self->device.vendor = g_ascii_strtoll(usbid[0], NULL, 16);
+    self->device.product = g_ascii_strtoll(usbid[1], NULL, 16);
     g_strfreev(usbid);
 
-    if (*vendor <= 0 || *vendor > 0xffff || *product < 0 || *product > 0xffff) {
-        g_printerr("Bad vendor:product values %04x:%04x", *vendor, *product);
+    if (self->device.vendor <= 0 || self->device.vendor > 0xffff ||
+        self->device.product < 0 || self->device.product > 0xffff) {
+        g_printerr("Bad vendor:product values %04x:%04x",
+                   self->device.vendor, self->device.product);
         return false;
     }
 
@@ -166,7 +151,7 @@ parse_opts(int *argc, char ***argv)
 
     self = g_new0(redirect, 1);
     self->watch_inout = true;
-    if (!parse_opt_device(device, &self->device.vendor, &self->device.product)) {
+    if (!parse_opt_device(self, device)) {
         g_printerr("Failed to parse device: '%s' - expected: vendor:product or busnum-devnum\n", device);
         g_clear_pointer(&self, g_free);
         goto end;
@@ -535,9 +520,16 @@ open_usb_device(redirect *self)
             continue;
         }
 
-        if (self->device.vendor != desc.idVendor ||
-            self->device.product != desc.idProduct) {
-            continue;
+        if (self->by_bus &&
+            (self->device.bus != libusb_get_bus_number(devs[i]) ||
+             self->device.device_number != libusb_get_device_address(devs[i]))) {
+             continue;
+        }
+
+        if (!self->by_bus &&
+            (self->device.vendor != desc.idVendor ||
+             self->device.product != desc.idProduct)) {
+             continue;
         }
 
         if (can_claim_usb_device(devs[i], &dev_handle)) {
@@ -674,6 +666,7 @@ main(int argc, char *argv[])
 
         socket_service = g_socket_service_new ();
         GInetAddress *iaddr = g_inet_address_new_loopback(G_SOCKET_FAMILY_IPV4);
+
         GSocketAddress *saddr = g_inet_socket_address_new(iaddr, self->port);
         g_object_unref(iaddr);
 
-- 
2.39.0