Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit cd35664c authored by Elliott Hughes's avatar Elliott Hughes
Browse files

Improve udev failure diagnostics.

A couple of folks had trouble understanding the existing message.

Before:

  8XV7N15917000596	no permissions (udev requires plugdev group membership); see [http://developer.android.com/tools/device.html]

After:

  8XV7N15917000596	no permissions (user buttmunch is not in the plugdev group); see [http://developer.android.com/tools/device.html]

This also fixes a libusb regression where we wouldn't show anything for
devices where we don't have permissions.

Bug: http://b/37707122
Test: ran "adb devices" as user buttmunch
Change-Id: I2fcd735ff4178145432b532a6e4dc8c93b2743fd
parent bdbcdf3a
Loading
Loading
Loading
Loading
+70 −37
Original line number Original line Diff line number Diff line
@@ -159,6 +159,20 @@ static std::string get_device_address(libusb_device* device) {
                        libusb_get_device_address(device));
                        libusb_get_device_address(device));
}
}


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;
}

static bool endpoint_is_output(uint8_t endpoint) {
static bool endpoint_is_output(uint8_t endpoint) {
    return (endpoint & LIBUSB_ENDPOINT_DIR_MASK) == LIBUSB_ENDPOINT_OUT;
    return (endpoint & LIBUSB_ENDPOINT_DIR_MASK) == LIBUSB_ENDPOINT_OUT;
}
}
@@ -291,15 +305,11 @@ static void poll_for_devices() {
                }
                }
            }
            }


            libusb_device_handle* handle_raw;
            bool writable = true;
            libusb_device_handle* handle_raw = nullptr;
            rc = libusb_open(device, &handle_raw);
            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);
            unique_device_handle handle(handle_raw);
            if (rc == 0) {
                LOG(DEBUG) << "successfully opened adb device at " << device_address << ", "
                LOG(DEBUG) << "successfully opened adb device at " << device_address << ", "
                           << StringPrintf("bulk_in = %#x, bulk_out = %#x", bulk_in, bulk_out);
                           << StringPrintf("bulk_in = %#x, bulk_out = %#x", bulk_in, bulk_out);


@@ -320,8 +330,8 @@ static void poll_for_devices() {
                // WARNING: this isn't released via RAII.
                // WARNING: this isn't released via RAII.
                rc = libusb_claim_interface(handle.get(), interface_num);
                rc = libusb_claim_interface(handle.get(), interface_num);
                if (rc != 0) {
                if (rc != 0) {
                LOG(WARNING) << "failed to claim adb interface for device '" << device_serial << "'"
                    LOG(WARNING) << "failed to claim adb interface for device '" << device_serial
                             << libusb_error_name(rc);
                                 << "'" << libusb_error_name(rc);
                    continue;
                    continue;
                }
                }


@@ -335,6 +345,28 @@ static void poll_for_devices() {
                        continue;
                        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,
            auto result = std::make_unique<usb_handle>(device_address, device_serial,
                                                       std::move(handle), interface_num, bulk_in,
                                                       std::move(handle), interface_num, bulk_in,
@@ -346,7 +378,8 @@ static void poll_for_devices() {
                usb_handles[device_address] = std::move(result);
                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 << "'";
            LOG(INFO) << "registered new usb device '" << device_serial << "'";
        }
        }
+23 −20
Original line number Original line Diff line number Diff line
@@ -25,13 +25,14 @@


#if defined(__linux__)
#if defined(__linux__)
#include <grp.h>
#include <grp.h>
#include <pwd.h>
#endif
#endif


static const char kPermissionsHelpUrl[] = "http://developer.android.com/tools/device.html";
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
// Returns a message describing any potential problems we find with udev, or an empty string if we
// find plugdev information (i.e. udev is not installed).
// can't find plugdev information (i.e. udev is not installed).
static const char* GetUdevProblem() {
static std::string GetUdevProblem() {
#if defined(__linux__)
#if defined(__linux__)
    errno = 0;
    errno = 0;
    group* plugdev_group = getgrnam("plugdev");
    group* plugdev_group = getgrnam("plugdev");
@@ -41,43 +42,45 @@ static const char* GetUdevProblem() {
            perror("failed to read plugdev group info");
            perror("failed to read plugdev group info");
        }
        }
        // We can't give any generally useful advice here, just let the caller print the help URL.
        // 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.
    // additionally just to be sure.
    if (group_member(plugdev_group->gr_gid) || getegid() == plugdev_group->gr_gid) {
    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.
        // 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
#else
    return nullptr;
    return "";
#endif
#endif
}
}


// Short help text must be a single line, and will look something like:
// 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 UsbNoPermissionsShortHelpText() {
    std::string help_text = "no permissions";
    std::string help_text = "no permissions";


    const char* problem = GetUdevProblem();
    std::string problem(GetUdevProblem());
    if (problem != nullptr) {
    if (!problem.empty()) help_text += " (" + problem + ")";
        help_text += android::base::StringPrintf(" (%s)", problem);
    }


    return android::base::StringPrintf("%s; see [%s]", help_text.c_str(), kPermissionsHelpUrl);
    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 UsbNoPermissionsLongHelpText() {
    std::string header = "insufficient permissions for device";
    std::string header = "insufficient permissions for device";


    const char* problem = GetUdevProblem();
    std::string problem(GetUdevProblem());
    if (problem != nullptr) {
    if (!problem.empty()) header += ": " + problem;
        header += android::base::StringPrintf(": %s", problem);
    }


    return android::base::StringPrintf("%s.\nSee [%s] for more information.",
    return android::base::StringPrintf("%s\nSee [%s] for more information", header.c_str(),
                                       header.c_str(), kPermissionsHelpUrl);
                                       kPermissionsHelpUrl);
}
}