Parse USB interface association descriptors.

device/usb/usb_descriptors.cc

Index: device/usb/usb_descriptors.cc
diff --git a/device/usb/usb_descriptors.cc b/device/usb/usb_descriptors.cc
index 6defda8ebfc76e8de4efa4d83763b6d0a80e61b8..22414d7abb4fe0930e01e5fd7bbfeaa006745a03 100644
--- a/device/usb/usb_descriptors.cc
+++ b/device/usb/usb_descriptors.cc
@@ -7,6 +7,7 @@
 #include <stddef.h>
 
 #include <algorithm>
+#include <vector>
 
 #include "base/barrier_closure.h"
 #include "base/bind.h"
@@ -26,6 +27,42 @@ const uint8_t kStringDescriptorType = 0x03;
 
 const int kControlTransferTimeout = 60000;  // 1 minute
 
+struct UsbInterfaceAssociationDescriptor {
+  UsbInterfaceAssociationDescriptor(uint8_t first_interface,
+                                    uint8_t interface_count)
+      : first_interface(first_interface), interface_count(interface_count) {}
+
+  bool operator<(const UsbInterfaceAssociationDescriptor& other) const {
+    return first_interface < other.first_interface;
+  }
+
+  uint8_t first_interface;
+  uint8_t interface_count;
+};
+
+void ParseInterfaceAssociationDescriptors(
+    const std::vector<uint8_t>& buffer,
+    std::vector<UsbInterfaceAssociationDescriptor>* functions) {
+  const uint8_t kInterfaceAssociationDescriptorType = 11;
+  const uint8_t kInterfaceAssociationDescriptorLength = 8;
+  std::vector<uint8_t>::const_iterator it = buffer.begin();
+
+  while (it != buffer.end()) {
+    // All descriptors must be at least 2 byte which means the length and type
+    // are safe to read.
+    if (std::distance(it, buffer.end()) < 2)
+      return;
+    uint8_t length = it[0];
+    if (length > std::distance(it, buffer.end()))
+      return;
+    if (it[1] == kInterfaceAssociationDescriptorType &&
+        length == kInterfaceAssociationDescriptorLength) {
+      functions->push_back(UsbInterfaceAssociationDescriptor(it[2], it[3]));
+    }
+    std::advance(it, length);
+  }
+}
+
 void StoreStringDescriptor(IndexMap::iterator it,
                            const base::Closure& callback,
                            const base::string16& string) {
@@ -113,7 +150,8 @@ UsbInterfaceDescriptor::UsbInterfaceDescriptor(uint8_t interface_number,
       alternate_setting(alternate_setting),
       interface_class(interface_class),
       interface_subclass(interface_subclass),
-      interface_protocol(interface_protocol) {}
+      interface_protocol(interface_protocol),
+      first_interface(interface_number) {}
 
 UsbInterfaceDescriptor::~UsbInterfaceDescriptor() = default;
 
@@ -128,6 +166,47 @@ UsbConfigDescriptor::UsbConfigDescriptor(uint8_t configuration_value,
 
 UsbConfigDescriptor::~UsbConfigDescriptor() = default;
 
+void UsbConfigDescriptor::AssignFirstInterfaceNumbers() {
+  std::vector<UsbInterfaceAssociationDescriptor> functions;
+  ParseInterfaceAssociationDescriptors(extra_data, &functions);
+  for (const auto& interface : interfaces) {
+    ParseInterfaceAssociationDescriptors(interface.extra_data, &functions);
+    for (const auto& endpoint : interface.endpoints)
+      ParseInterfaceAssociationDescriptors(endpoint.extra_data, &functions);
+  }
+
+  // libusb has collected interface association descriptors in the |extra_data|
+  // fields of other descriptor types. This may have disturbed their order
+  // but sorting by the bFirstInterface should fix it.
+  std::sort(functions.begin(), functions.end());
+
+  uint8_t remaining_interfaces = 0;
+  auto function_it = functions.cbegin();
+  for (auto interface_it = interfaces.begin(); interface_it != interfaces.end();
+       ++interface_it) {
+    if (remaining_interfaces > 0) {
+      // Continuation of a previous function. Tag all alternate interfaces
+      // (which are guaranteed to be contiguous).
+      for (uint8_t interface_number = interface_it->interface_number;
+           interface_it != interfaces.end() &&

[gab, 2016/02/14 21:02:17]

This is wrong as if the inner loop exits because this hit the end, the outer
loop will proceed with another increment prior to verifying its condition and
thus will go past the end.

http://crbug.com/586824

I will revert this CL for now as it otherwise makes Chrome hang when using USB
devices. Hopefully tomorrow's Canary picks up the revert.

Also: feels like this should be unit-tested.

[gab, 2016/02/14 21:10:06]

Apologies, looks like there is a unit test, but it doesn't trigger this issue,
should probably add a case that hits the end in the inner-loop.

Cheers!

+           interface_it->interface_number == interface_number;
+           ++interface_it) {
+        interface_it->first_interface = function_it->first_interface;
+      }
+      if (--remaining_interfaces == 0)
+        ++function_it;
+    } else if (function_it != functions.end() &&
+               interface_it->interface_number == function_it->first_interface) {
+      // Start of a new function.
+      interface_it->first_interface = function_it->first_interface;
+      remaining_interfaces = function_it->interface_count - 1;
+    } else {
+      // Unassociated interfaces already have |first_interface| set to
+      // |interface_number|.
+    }
+  }
+}
+
 bool ParseUsbStringDescriptor(const std::vector<uint8_t>& descriptor,
                               base::string16* output) {
   if (descriptor.size() < 2 || descriptor[1] != kStringDescriptorType)