From 1b708d368f29e6053064c9cf6949ab6ebdbb7ac5 Mon Sep 17 00:00:00 2001 From: Elliott Hughes Date: Fri, 11 Dec 2015 19:07:01 -0800 Subject: [PATCH] Share the new adb USB diagnostic code with fastboot. Bug: http://b/26134129 Change-Id: Ieaf0651c7b3f8a028760982091ec63a21a5484ba --- adb/Android.mk | 15 ++++++++ adb/adb.h | 5 --- adb/diagnose_usb.cpp | 83 ++++++++++++++++++++++++++++++++++++++++ adb/diagnose_usb.h | 27 +++++++++++++ adb/transport.cpp | 12 ++---- adb/usb_linux.cpp | 52 ------------------------- adb/usb_linux_client.cpp | 9 ----- adb/usb_osx.cpp | 9 ----- adb/usb_windows.cpp | 9 ----- fastboot/Android.mk | 13 ++++--- fastboot/fastboot.cpp | 13 ++++--- 11 files changed, 144 insertions(+), 103 deletions(-) create mode 100644 adb/diagnose_usb.cpp create mode 100644 adb/diagnose_usb.h diff --git a/adb/Android.mk b/adb/Android.mk index c38cf9375..fe3c9ccbf 100644 --- a/adb/Android.mk +++ b/adb/Android.mk @@ -160,6 +160,19 @@ LOCAL_STATIC_LIBRARIES := libadbd LOCAL_SHARED_LIBRARIES := libbase libcutils include $(BUILD_NATIVE_TEST) +# libdiagnose_usb +# ========================================================= + +include $(CLEAR_VARS) +LOCAL_MODULE := libdiagnose_usb +LOCAL_MODULE_HOST_OS := darwin linux windows +LOCAL_CFLAGS := $(LIBADB_CFLAGS) +LOCAL_SRC_FILES := diagnose_usb.cpp +# Even though we're building a static library (and thus there's no link step for +# this to take effect), this adds the includes to our path. +LOCAL_STATIC_LIBRARIES := libbase +include $(BUILD_HOST_STATIC_LIBRARY) + # adb_test # ========================================================= @@ -185,6 +198,7 @@ LOCAL_STATIC_LIBRARIES := \ libadb \ libcrypto_static \ libcutils \ + libdiagnose_usb \ # Set entrypoint to wmain from sysdeps_win32.cpp instead of main LOCAL_LDFLAGS_windows := -municode @@ -261,6 +275,7 @@ LOCAL_STATIC_LIBRARIES := \ libadb \ libbase \ libcrypto_static \ + libdiagnose_usb \ liblog \ # Don't use libcutils on Windows. diff --git a/adb/adb.h b/adb/adb.h index 774215ef4..9020fc3a3 100644 --- a/adb/adb.h +++ b/adb/adb.h @@ -226,11 +226,6 @@ void usb_kick(usb_handle *h); int is_adb_interface(int vid, int pid, int usb_class, int usb_subclass, int usb_protocol); #endif -// USB permission error help text. The short version will be one line, long may be multi-line. -// Returns a string message to print, or an empty string if no problems could be found. -std::string UsbNoPermissionsShortHelpText(); -std::string UsbNoPermissionsLongHelpText(); - int adb_commandline(int argc, const char **argv); ConnectionState connection_state(atransport *t); diff --git a/adb/diagnose_usb.cpp b/adb/diagnose_usb.cpp new file mode 100644 index 000000000..0f067b0ec --- /dev/null +++ b/adb/diagnose_usb.cpp @@ -0,0 +1,83 @@ +/* + * Copyright (C) 2015 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "diagnose_usb.h" + +#include +#include + +#include + +#include + +#if defined(__linux__) +#include +#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() { +#if defined(__linux__) + errno = 0; + group* plugdev_group = getgrnam("plugdev"); + + if (plugdev_group == nullptr) { + if (errno != 0) { + 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; + } + + // getgroups(2) indicates that the group_member() 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 "udev requires plugdev group membership"; +#else + return nullptr; +#endif +} + +// Short help text must be a single line, and will look something like: +// no permissions (reason); see +std::string UsbNoPermissionsShortHelpText() { + std::string help_text = "no permissions"; + + const char* problem = GetUdevProblem(); + if (problem != nullptr) { + help_text += android::base::StringPrintf(" (%s)", 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. +std::string UsbNoPermissionsLongHelpText() { + std::string header = "insufficient permissions for device"; + + const char* problem = GetUdevProblem(); + if (problem != nullptr) { + header += android::base::StringPrintf(": %s", problem); + } + + return android::base::StringPrintf("%s.\nSee [%s] for more information.", + header.c_str(), kPermissionsHelpUrl); +} diff --git a/adb/diagnose_usb.h b/adb/diagnose_usb.h new file mode 100644 index 000000000..325b2e3b6 --- /dev/null +++ b/adb/diagnose_usb.h @@ -0,0 +1,27 @@ +/* + * Copyright (C) 2015 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#ifndef __DIAGNOSE_LINUX_USB_H +#define __DIAGNOSE_LINUX_USB_H + +#include + +// USB permission error help text. The short version will be one line, long may be multi-line. +// Returns a string message to print, or an empty string if no problems could be found. +std::string UsbNoPermissionsShortHelpText(); +std::string UsbNoPermissionsLongHelpText(); + +#endif diff --git a/adb/transport.cpp b/adb/transport.cpp index d4f60ece4..6020ad5b1 100644 --- a/adb/transport.cpp +++ b/adb/transport.cpp @@ -35,6 +35,7 @@ #include "adb.h" #include "adb_utils.h" +#include "diagnose_usb.h" static void transport_unref(atransport *t); @@ -674,11 +675,9 @@ atransport* acquire_one_transport(TransportType type, const char* serial, adb_mutex_lock(&transport_lock); for (const auto& t : transport_list) { if (t->connection_state == kCsNoPerm) { +#if ADB_HOST *error_out = UsbNoPermissionsLongHelpText(); - // If we couldn't figure out a reasonable help message default to something generic. - if (error_out->empty()) { - *error_out = "insufficient permissions for device"; - } +#endif continue; } @@ -759,10 +758,7 @@ const std::string atransport::connection_state_name() const { case kCsDevice: return "device"; case kCsHost: return "host"; case kCsRecovery: return "recovery"; - case kCsNoPerm: { - std::string message = UsbNoPermissionsShortHelpText(); - return message.empty() ? "no permissions" : message; - } + case kCsNoPerm: return UsbNoPermissionsShortHelpText(); case kCsSideload: return "sideload"; case kCsUnauthorized: return "unauthorized"; default: return "unknown"; diff --git a/adb/usb_linux.cpp b/adb/usb_linux.cpp index e887a94d2..ed5d2d67e 100644 --- a/adb/usb_linux.cpp +++ b/adb/usb_linux.cpp @@ -22,7 +22,6 @@ #include #include #include -#include #include #include #include @@ -596,54 +595,3 @@ void usb_init() { fatal_errno("cannot create device_poll thread"); } } - -static const char kPermissionsHelpUrl[] = "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() { - errno = 0; - group* plugdev_group = getgrnam("plugdev"); - - if (plugdev_group == nullptr) { - if (errno != 0) { - D("failed to read plugdev group info: %s", strerror(errno)); - } - // We can't give any generally useful advice here, just let the caller print the help URL. - return nullptr; - } - - // getgroups(2) indicates that the group_member() 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 "udev requires plugdev group membership"; -} - -// Short help text must be a single line, and will look something like: -// no permissions (reason); see -std::string UsbNoPermissionsShortHelpText() { - std::string help_text = "no permissions"; - - const char* problem = GetUdevProblem(); - if (problem != nullptr) { - help_text += android::base::StringPrintf(" (%s)", 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. -std::string UsbNoPermissionsLongHelpText() { - std::string header = "USB permission failure"; - - const char* problem = GetUdevProblem(); - if (problem != nullptr) { - header += android::base::StringPrintf(": %s", problem); - } - - return android::base::StringPrintf("%s.\nSee [%s] for more information.", - header.c_str(), kPermissionsHelpUrl); -} diff --git a/adb/usb_linux_client.cpp b/adb/usb_linux_client.cpp index c5e7452f1..ceed8facf 100644 --- a/adb/usb_linux_client.cpp +++ b/adb/usb_linux_client.cpp @@ -571,12 +571,3 @@ void usb_kick(usb_handle *h) { h->kick(h); } - -// kCsNoPerm is a host-side issue, we can just ignore it here. -std::string UsbNoPermissionsShortHelpText() { - return ""; -} - -std::string UsbNoPermissionsLongHelpText() { - return ""; -} diff --git a/adb/usb_osx.cpp b/adb/usb_osx.cpp index f49479529..148be1d78 100644 --- a/adb/usb_osx.cpp +++ b/adb/usb_osx.cpp @@ -552,12 +552,3 @@ void usb_kick(usb_handle *handle) handle->interface = 0; } } - -// kCsNoPerm is Linux-only. -std::string UsbNoPermissionsShortHelpText() { - return ""; -} - -std::string UsbNoPermissionsLongHelpText() { - return ""; -} diff --git a/adb/usb_windows.cpp b/adb/usb_windows.cpp index 91246852f..8d3501e24 100644 --- a/adb/usb_windows.cpp +++ b/adb/usb_windows.cpp @@ -659,12 +659,3 @@ static void kick_devices() { } adb_mutex_unlock(&usb_lock); } - -// kCsNoPerm is Linux-only. -std::string UsbNoPermissionsShortHelpText() { - return ""; -} - -std::string UsbNoPermissionsLongHelpText() { - return ""; -} diff --git a/fastboot/Android.mk b/fastboot/Android.mk index 5ca88cb3e..c293b578a 100644 --- a/fastboot/Android.mk +++ b/fastboot/Android.mk @@ -18,9 +18,12 @@ fastboot_version := $(shell git -C $(LOCAL_PATH) rev-parse --short=12 HEAD 2>/de include $(CLEAR_VARS) -LOCAL_C_INCLUDES := $(LOCAL_PATH)/../mkbootimg \ +LOCAL_C_INCLUDES := \ + $(LOCAL_PATH)/../adb \ + $(LOCAL_PATH)/../mkbootimg \ $(LOCAL_PATH)/../../extras/ext4_utils \ - $(LOCAL_PATH)/../../extras/f2fs_utils + $(LOCAL_PATH)/../../extras/f2fs_utils \ + LOCAL_SRC_FILES := protocol.cpp engine.cpp bootimg_utils.cpp fastboot.cpp util.cpp fs.cpp LOCAL_MODULE := fastboot LOCAL_MODULE_TAGS := debug @@ -31,8 +34,10 @@ LOCAL_CFLAGS += -Wall -Wextra -Werror -Wunreachable-code LOCAL_CFLAGS += -DFASTBOOT_REVISION='"$(fastboot_version)"' LOCAL_SRC_FILES_linux := usb_linux.cpp util_linux.cpp +LOCAL_STATIC_LIBRARIES_linux := libselinux LOCAL_SRC_FILES_darwin := usb_osx.cpp util_osx.cpp +LOCAL_STATIC_LIBRARIES_darwin := libselinux LOCAL_LDLIBS_darwin := -lpthread -framework CoreFoundation -framework IOKit -framework Carbon LOCAL_CFLAGS_darwin := -Wno-unused-parameter @@ -49,11 +54,9 @@ LOCAL_STATIC_LIBRARIES := \ libutils \ liblog \ libz \ + libdiagnose_usb \ libbase \ -LOCAL_STATIC_LIBRARIES_darwin := libselinux -LOCAL_STATIC_LIBRARIES_linux := libselinux - # libf2fs_dlutils_host will dlopen("libf2fs_fmt_host_dyn") LOCAL_CFLAGS_linux := -DUSE_F2FS LOCAL_LDFLAGS_linux := -ldl -rdynamic -Wl,-rpath,. diff --git a/fastboot/fastboot.cpp b/fastboot/fastboot.cpp index 1ab943605..5b663668e 100644 --- a/fastboot/fastboot.cpp +++ b/fastboot/fastboot.cpp @@ -53,6 +53,7 @@ #include #include "bootimg_utils.h" +#include "diagnose_usb.h" #include "fastboot.h" #include "fs.h" #include "transport.h" @@ -204,21 +205,21 @@ static int match_fastboot(usb_ifc_info* info) { static int list_devices_callback(usb_ifc_info* info) { if (match_fastboot_with_serial(info, nullptr) == 0) { - const char* serial = info->serial_number; + std::string serial = info->serial_number; if (!info->writable) { - serial = "no permissions"; // like "adb devices" + serial = UsbNoPermissionsShortHelpText(); } if (!serial[0]) { serial = "????????????"; } // output compatible with "adb devices" if (!long_listing) { - printf("%s\tfastboot\n", serial); - } else if (strcmp("", info->device_path) == 0) { - printf("%-22s fastboot\n", serial); + printf("%s\tfastboot", serial.c_str()); } else { - printf("%-22s fastboot %s\n", serial, info->device_path); + printf("%-22s fastboot", serial.c_str()); + if (strlen(info->device_path) > 0) printf(" %s", info->device_path); } + putchar('\n'); } return -1;