Merge "Remove unused lambda captures"
diff --git a/adb/client/usb_libusb.cpp b/adb/client/usb_libusb.cpp
index 20610ee..7539e5a 100644
--- a/adb/client/usb_libusb.cpp
+++ b/adb/client/usb_libusb.cpp
@@ -159,6 +159,22 @@
libusb_get_device_address(device));
}
+#if defined(__linux__)
+static std::string get_device_serial_path(libusb_device* device) {
+ uint8_t ports[7];
+ int port_count = libusb_get_port_numbers(device, ports, 7);
+ if (port_count < 0) return "";
+
+ std::string path =
+ StringPrintf("/sys/bus/usb/devices/%d-%d", libusb_get_bus_number(device), ports[0]);
+ for (int port = 1; port < port_count; ++port) {
+ path += StringPrintf(".%d", ports[port]);
+ }
+ path += "/serial";
+ return path;
+}
+#endif
+
static bool endpoint_is_output(uint8_t endpoint) {
return (endpoint & LIBUSB_ENDPOINT_DIR_MASK) == LIBUSB_ENDPOINT_OUT;
}
@@ -291,49 +307,67 @@
}
}
- libusb_device_handle* handle_raw;
+ bool writable = true;
+ libusb_device_handle* handle_raw = nullptr;
rc = libusb_open(device, &handle_raw);
- if (rc != 0) {
- LOG(WARNING) << "failed to open usb device at " << device_address << ": "
- << libusb_error_name(rc);
- continue;
- }
-
unique_device_handle handle(handle_raw);
- LOG(DEBUG) << "successfully opened adb device at " << device_address << ", "
- << StringPrintf("bulk_in = %#x, bulk_out = %#x", bulk_in, bulk_out);
-
- device_serial.resize(255);
- rc = libusb_get_string_descriptor_ascii(
- handle_raw, device_desc.iSerialNumber,
- reinterpret_cast<unsigned char*>(&device_serial[0]), device_serial.length());
if (rc == 0) {
- LOG(WARNING) << "received empty serial from device at " << device_address;
- continue;
- } else if (rc < 0) {
- LOG(WARNING) << "failed to get serial from device at " << device_address
- << libusb_error_name(rc);
- continue;
- }
- device_serial.resize(rc);
+ LOG(DEBUG) << "successfully opened adb device at " << device_address << ", "
+ << StringPrintf("bulk_in = %#x, bulk_out = %#x", bulk_in, bulk_out);
- // WARNING: this isn't released via RAII.
- rc = libusb_claim_interface(handle.get(), interface_num);
- if (rc != 0) {
- LOG(WARNING) << "failed to claim adb interface for device '" << device_serial << "'"
- << libusb_error_name(rc);
- continue;
- }
-
- for (uint8_t endpoint : {bulk_in, bulk_out}) {
- rc = libusb_clear_halt(handle.get(), endpoint);
- if (rc != 0) {
- LOG(WARNING) << "failed to clear halt on device '" << device_serial
- << "' endpoint 0x" << std::hex << endpoint << ": "
+ device_serial.resize(255);
+ rc = libusb_get_string_descriptor_ascii(
+ handle_raw, device_desc.iSerialNumber,
+ reinterpret_cast<unsigned char*>(&device_serial[0]), device_serial.length());
+ if (rc == 0) {
+ LOG(WARNING) << "received empty serial from device at " << device_address;
+ continue;
+ } else if (rc < 0) {
+ LOG(WARNING) << "failed to get serial from device at " << device_address
<< libusb_error_name(rc);
- libusb_release_interface(handle.get(), interface_num);
continue;
}
+ device_serial.resize(rc);
+
+ // WARNING: this isn't released via RAII.
+ rc = libusb_claim_interface(handle.get(), interface_num);
+ if (rc != 0) {
+ LOG(WARNING) << "failed to claim adb interface for device '" << device_serial
+ << "'" << libusb_error_name(rc);
+ continue;
+ }
+
+ for (uint8_t endpoint : {bulk_in, bulk_out}) {
+ rc = libusb_clear_halt(handle.get(), endpoint);
+ if (rc != 0) {
+ LOG(WARNING) << "failed to clear halt on device '" << device_serial
+ << "' endpoint 0x" << std::hex << endpoint << ": "
+ << libusb_error_name(rc);
+ libusb_release_interface(handle.get(), interface_num);
+ continue;
+ }
+ }
+ } else {
+ LOG(WARNING) << "failed to open usb device at " << device_address << ": "
+ << libusb_error_name(rc);
+ writable = false;
+
+#if defined(__linux__)
+ // libusb doesn't think we should be messing around with devices we don't have
+ // write access to, but Linux at least lets us get the serial number anyway.
+ if (!android::base::ReadFileToString(get_device_serial_path(device),
+ &device_serial)) {
+ // We don't actually want to treat an unknown serial as an error because
+ // devices aren't able to communicate a serial number in early bringup.
+ // http://b/20883914
+ device_serial = "unknown";
+ }
+ device_serial = android::base::Trim(device_serial);
+#else
+ // On Mac OS and Windows, we're screwed. But I don't think this situation actually
+ // happens on those OSes.
+ continue;
+#endif
}
auto result = std::make_unique<usb_handle>(device_address, device_serial,
@@ -346,7 +380,8 @@
usb_handles[device_address] = std::move(result);
}
- register_usb_transport(usb_handle_raw, device_serial.c_str(), device_address.c_str(), 1);
+ register_usb_transport(usb_handle_raw, device_serial.c_str(), device_address.c_str(),
+ writable);
LOG(INFO) << "registered new usb device '" << device_serial << "'";
}
diff --git a/adb/diagnose_usb.cpp b/adb/diagnose_usb.cpp
index 0f067b0..9f721bf 100644
--- a/adb/diagnose_usb.cpp
+++ b/adb/diagnose_usb.cpp
@@ -25,13 +25,14 @@
#if defined(__linux__)
#include <grp.h>
+#include <pwd.h>
#endif
static const char kPermissionsHelpUrl[] = "http://developer.android.com/tools/device.html";
-// Returns a message describing any potential problems we find with udev, or nullptr if we can't
-// find plugdev information (i.e. udev is not installed).
-static const char* GetUdevProblem() {
+// Returns a message describing any potential problems we find with udev, or an empty string if we
+// can't find plugdev information (i.e. udev is not installed).
+static std::string GetUdevProblem() {
#if defined(__linux__)
errno = 0;
group* plugdev_group = getgrnam("plugdev");
@@ -41,43 +42,45 @@
perror("failed to read plugdev group info");
}
// We can't give any generally useful advice here, just let the caller print the help URL.
- return nullptr;
+ return "";
}
- // getgroups(2) indicates that the group_member() may not check the egid so we check it
+ // getgroups(2) indicates that the GNU group_member(3) may not check the egid so we check it
// additionally just to be sure.
if (group_member(plugdev_group->gr_gid) || getegid() == plugdev_group->gr_gid) {
// The user is in plugdev so the problem is likely with the udev rules.
- return "verify udev rules";
+ return "user in plugdev group; are your udev rules wrong?";
}
- return "udev requires plugdev group membership";
+ passwd* pwd = getpwuid(getuid());
+ return android::base::StringPrintf("user %s is not in the plugdev group",
+ pwd ? pwd->pw_name : "?");
#else
- return nullptr;
+ return "";
#endif
}
// Short help text must be a single line, and will look something like:
-// no permissions (reason); see <URL>
+//
+// no permissions (reason); see [URL]
std::string UsbNoPermissionsShortHelpText() {
std::string help_text = "no permissions";
- const char* problem = GetUdevProblem();
- if (problem != nullptr) {
- help_text += android::base::StringPrintf(" (%s)", problem);
- }
+ std::string problem(GetUdevProblem());
+ if (!problem.empty()) help_text += " (" + problem + ")";
return android::base::StringPrintf("%s; see [%s]", help_text.c_str(), kPermissionsHelpUrl);
}
-// Long help text can span multiple lines and should provide more detailed information.
+// Long help text can span multiple lines but doesn't currently provide more detailed information:
+//
+// insufficient permissions for device: reason
+// See [URL] for more information
std::string UsbNoPermissionsLongHelpText() {
std::string header = "insufficient permissions for device";
- const char* problem = GetUdevProblem();
- if (problem != nullptr) {
- header += android::base::StringPrintf(": %s", problem);
- }
+ std::string problem(GetUdevProblem());
+ if (!problem.empty()) header += ": " + problem;
- return android::base::StringPrintf("%s.\nSee [%s] for more information.",
- header.c_str(), kPermissionsHelpUrl);
+ return android::base::StringPrintf("%s\nSee [%s] for more information", header.c_str(),
+ kPermissionsHelpUrl);
}
diff --git a/libcutils/tests/AshmemTest.cpp b/libcutils/tests/AshmemTest.cpp
index 51c679f..a87e23e 100644
--- a/libcutils/tests/AshmemTest.cpp
+++ b/libcutils/tests/AshmemTest.cpp
@@ -14,10 +14,11 @@
* limitations under the License.
*/
-#include <sys/mman.h>
+#include <android-base/unique_fd.h>
#include <cutils/ashmem.h>
#include <gtest/gtest.h>
-#include <android-base/unique_fd.h>
+#include <linux/fs.h>
+#include <sys/mman.h>
using android::base::unique_fd;
@@ -29,8 +30,8 @@
ASSERT_EQ(0, ashmem_set_prot_region(fd, prot));
}
-void TestMmap(const unique_fd &fd, size_t size, int prot, void **region) {
- *region = mmap(nullptr, size, prot, MAP_SHARED, fd, 0);
+void TestMmap(const unique_fd& fd, size_t size, int prot, void** region, off_t off = 0) {
+ *region = mmap(nullptr, size, prot, MAP_SHARED, fd, off);
ASSERT_NE(MAP_FAILED, *region);
}
@@ -38,6 +39,10 @@
EXPECT_EQ(MAP_FAILED, mmap(nullptr, size, prot, MAP_SHARED, fd, 0));
}
+void TestProtIs(const unique_fd& fd, int prot) {
+ EXPECT_EQ(prot, ioctl(fd, ASHMEM_GET_PROT_MASK));
+}
+
void FillData(uint8_t* data, size_t dataLen) {
for (size_t i = 0; i < dataLen; i++) {
data[i] = i & 0xFF;
@@ -101,6 +106,63 @@
EXPECT_EQ(0, munmap(region2, size));
}
+TEST(AshmemTest, FileOperationsTest) {
+ unique_fd fd;
+ void* region;
+
+ // Allocate a 4-page buffer, but leave page-sized holes on either side
+ constexpr size_t size = PAGE_SIZE * 4;
+ constexpr size_t dataSize = PAGE_SIZE * 2;
+ constexpr size_t holeSize = PAGE_SIZE;
+ ASSERT_NO_FATAL_FAILURE(TestCreateRegion(size, fd, PROT_READ | PROT_WRITE));
+ ASSERT_NO_FATAL_FAILURE(TestMmap(fd, dataSize, PROT_READ | PROT_WRITE, ®ion, holeSize));
+
+ uint8_t data[dataSize];
+ FillData(data, dataSize);
+ memcpy(region, data, dataSize);
+
+ constexpr off_t dataStart = holeSize;
+ constexpr off_t dataEnd = dataStart + dataSize;
+
+ // The sequence of seeks below looks something like this:
+ //
+ // [ ][data][data][ ]
+ // --^ lseek(99, SEEK_SET)
+ // ------^ lseek(dataStart, SEEK_CUR)
+ // ------^ lseek(0, SEEK_DATA)
+ // ------------^ lseek(dataStart, SEEK_HOLE)
+ // ^-- lseek(-99, SEEK_END)
+ // ^------ lseek(-dataStart, SEEK_CUR)
+ const struct {
+ // lseek() parameters
+ off_t offset;
+ int whence;
+ // Expected lseek() return value
+ off_t ret;
+ } seeks[] = {
+ {99, SEEK_SET, 99}, {dataStart, SEEK_CUR, dataStart + 99},
+ {0, SEEK_DATA, dataStart}, {dataStart, SEEK_HOLE, dataEnd},
+ {-99, SEEK_END, size - 99}, {-dataStart, SEEK_CUR, dataEnd - 99},
+ };
+ for (const auto& cfg : seeks) {
+ errno = 0;
+ auto off = lseek(fd, cfg.offset, cfg.whence);
+ ASSERT_EQ(cfg.ret, off) << "lseek(" << cfg.offset << ", " << cfg.whence << ") failed"
+ << (errno ? ": " : "") << (errno ? strerror(errno) : "");
+
+ if (off >= dataStart && off < dataEnd) {
+ off_t dataOff = off - dataStart;
+ ssize_t readSize = dataSize - dataOff;
+ uint8_t buf[readSize];
+
+ ASSERT_EQ(readSize, TEMP_FAILURE_RETRY(read(fd, buf, readSize)));
+ EXPECT_EQ(0, memcmp(buf, data + dataOff, readSize));
+ }
+ }
+
+ EXPECT_EQ(0, munmap(region, dataSize));
+}
+
TEST(AshmemTest, ProtTest) {
unique_fd fd;
constexpr size_t size = PAGE_SIZE;
@@ -108,13 +170,25 @@
ASSERT_NO_FATAL_FAILURE(TestCreateRegion(size, fd, PROT_READ));
TestProtDenied(fd, size, PROT_WRITE);
+ TestProtIs(fd, PROT_READ);
ASSERT_NO_FATAL_FAILURE(TestMmap(fd, size, PROT_READ, ®ion));
EXPECT_EQ(0, munmap(region, size));
ASSERT_NO_FATAL_FAILURE(TestCreateRegion(size, fd, PROT_WRITE));
TestProtDenied(fd, size, PROT_READ);
+ TestProtIs(fd, PROT_WRITE);
ASSERT_NO_FATAL_FAILURE(TestMmap(fd, size, PROT_WRITE, ®ion));
EXPECT_EQ(0, munmap(region, size));
+
+ ASSERT_NO_FATAL_FAILURE(TestCreateRegion(size, fd, PROT_READ | PROT_WRITE));
+ TestProtIs(fd, PROT_READ | PROT_WRITE);
+ ASSERT_EQ(0, ashmem_set_prot_region(fd, PROT_READ));
+ errno = 0;
+ ASSERT_EQ(-1, ashmem_set_prot_region(fd, PROT_READ | PROT_WRITE))
+ << "kernel shouldn't allow adding protection bits";
+ EXPECT_EQ(EINVAL, errno);
+ TestProtIs(fd, PROT_READ);
+ TestProtDenied(fd, size, PROT_WRITE);
}
TEST(AshmemTest, ForkProtTest) {