From 2ea46521f3c6a275b100737ea071ecd473a38aaa Mon Sep 17 00:00:00 2001 From: Josh Gao Date: Tue, 10 Apr 2018 14:35:06 -0700 Subject: [PATCH] adb: properly calculate packet size on Mac. OS X reports maxPacketSize as wMaxPacketSize * (bMaxBurst + 1) in the deprecated GetPipeProperties function. Use the also-deprecated GetPipePropetiesV2 API to get bMaxBurst and figure out what wMaxPacketSize is. (This file is going to go away eventually, so don't bother with switching to the recommended GetPipePropertiesV3, since it would be a substantially larger charge.) libusb is unaffected. Bug: http://b/77733422 Test: python test_device.py Change-Id: I66517d699a4f39b93ba5eb7882bd8ee6c70f3672 --- adb/client/usb_osx.cpp | 33 ++++++++++++++++++++++----------- adb/test_device.py | 18 ++++++++++-------- 2 files changed, 32 insertions(+), 19 deletions(-) diff --git a/adb/client/usb_osx.cpp b/adb/client/usb_osx.cpp index b15d28a27..8a95a1907 100644 --- a/adb/client/usb_osx.cpp +++ b/adb/client/usb_osx.cpp @@ -50,7 +50,7 @@ struct usb_handle { UInt8 bulkIn; UInt8 bulkOut; - IOUSBInterfaceInterface190** interface; + IOUSBInterfaceInterface550** interface; unsigned int zero_mask; size_t max_packet_size; @@ -106,8 +106,8 @@ static void AddDevice(std::unique_ptr handle) { } static void AndroidInterfaceAdded(io_iterator_t iterator); -static std::unique_ptr CheckInterface(IOUSBInterfaceInterface190 **iface, - UInt16 vendor, UInt16 product); +static std::unique_ptr CheckInterface(IOUSBInterfaceInterface550** iface, UInt16 vendor, + UInt16 product); static bool FindUSBDevices() { // Create the matching dictionary to find the Android device's adb interface. @@ -295,8 +295,8 @@ AndroidInterfaceAdded(io_iterator_t iterator) continue; } - std::unique_ptr handle = CheckInterface((IOUSBInterfaceInterface190**)iface, - vendor, product); + std::unique_ptr handle = + CheckInterface((IOUSBInterfaceInterface550**)iface, vendor, product); if (handle == nullptr) { LOG(ERROR) << "Could not find device interface"; (*iface)->Release(iface); @@ -315,7 +315,7 @@ AndroidInterfaceAdded(io_iterator_t iterator) // Used to clear both the endpoints before starting. // When adb quits, we might clear the host endpoint but not the device. // So we make sure both sides are clear before starting up. -static bool ClearPipeStallBothEnds(IOUSBInterfaceInterface190** interface, UInt8 bulkEp) { +static bool ClearPipeStallBothEnds(IOUSBInterfaceInterface550** interface, UInt8 bulkEp) { IOReturn rc = (*interface)->ClearPipeStallBothEnds(interface, bulkEp); if (rc != kIOReturnSuccess) { LOG(ERROR) << "Could not clear pipe stall both ends: " << std::hex << rc; @@ -326,9 +326,8 @@ static bool ClearPipeStallBothEnds(IOUSBInterfaceInterface190** interface, UInt8 //* TODO: simplify this further since we only register to get ADB interface //* subclass+protocol events -static std::unique_ptr -CheckInterface(IOUSBInterfaceInterface190 **interface, UInt16 vendor, UInt16 product) -{ +static std::unique_ptr CheckInterface(IOUSBInterfaceInterface550** interface, + UInt16 vendor, UInt16 product) { std::unique_ptr handle; IOReturn kr; UInt8 interfaceNumEndpoints, interfaceClass, interfaceSubClass, interfaceProtocol; @@ -376,9 +375,14 @@ CheckInterface(IOUSBInterfaceInterface190 **interface, UInt16 vendor, UInt16 pro UInt8 interval; UInt8 number; UInt8 direction; + UInt8 maxBurst; + UInt8 mult; + UInt16 bytesPerInterval; - kr = (*interface)->GetPipeProperties(interface, endpoint, &direction, - &number, &transferType, &maxPacketSize, &interval); + kr = (*interface) + ->GetPipePropertiesV2(interface, endpoint, &direction, &number, &transferType, + &maxPacketSize, &interval, &maxBurst, &mult, + &bytesPerInterval); if (kr != kIOReturnSuccess) { LOG(ERROR) << "FindDeviceInterface - could not get pipe properties: " << std::hex << kr; @@ -397,6 +401,13 @@ CheckInterface(IOUSBInterfaceInterface190 **interface, UInt16 vendor, UInt16 pro if (!ClearPipeStallBothEnds(interface, handle->bulkOut)) goto err_get_pipe_props; } + if (maxBurst != 0) + // bMaxBurst is the number of additional packets in the burst. + maxPacketSize /= (maxBurst + 1); + + // mult is only relevant for isochronous endpoints. + CHECK_EQ(0, mult); + handle->zero_mask = maxPacketSize - 1; handle->max_packet_size = maxPacketSize; } diff --git a/adb/test_device.py b/adb/test_device.py index d39eb144a..b1ad04344 100644 --- a/adb/test_device.py +++ b/adb/test_device.py @@ -1275,16 +1275,18 @@ class DeviceOfflineTest(DeviceTest): """ # The values that trigger things are 507 (512 - 5 bytes from shell protocol) + 1024*n # Probe some surrounding values as well, for the hell of it. - for length in [506, 507, 508, 1018, 1019, 1020, 1530, 1531, 1532]: - cmd = ['dd', 'if=/dev/zero', 'bs={}'.format(length), 'count=1', '2>/dev/null;' - 'echo', 'foo'] - rc, stdout, _ = self.device.shell_nocheck(cmd) + for base in [512] + range(1024, 1024 * 16, 1024): + for offset in [-6, -5, -4]: + length = base + offset + cmd = ['dd', 'if=/dev/zero', 'bs={}'.format(length), 'count=1', '2>/dev/null;' + 'echo', 'foo'] + rc, stdout, _ = self.device.shell_nocheck(cmd) - self.assertEqual(0, rc) + self.assertEqual(0, rc) - # Output should be '\0' * length, followed by "foo\n" - self.assertEqual(length, len(stdout) - 4) - self.assertEqual(stdout, "\0" * length + "foo\n") + # Output should be '\0' * length, followed by "foo\n" + self.assertEqual(length, len(stdout) - 4) + self.assertEqual(stdout, "\0" * length + "foo\n") def main():