adb: fix feature sets on devices that don't have them.
Previously, we weren't distinguishing between nonexistent devices and
devices that don't support features, resulting in lots of stuff being
broken for pre-N devices.
Bug: http://b/154272158
Test: adb shell on a JB emulator
Change-Id: I0058b2a60678e1ad0503f5abac30157be976c9c4
diff --git a/adb/client/adb_client.cpp b/adb/client/adb_client.cpp
index 0ad0465..31c938c 100644
--- a/adb/client/adb_client.cpp
+++ b/adb/client/adb_client.cpp
@@ -416,14 +416,18 @@
return android::base::StringPrintf("%s:%s", prefix, command);
}
-const FeatureSet& adb_get_feature_set() {
- static const android::base::NoDestructor<FeatureSet> features([] {
- std::string result;
- if (!adb_query(format_host_command("features"), &result, &result)) {
- fprintf(stderr, "failed to get feature set: %s\n", result.c_str());
- return FeatureSet{};
- }
- return StringToFeatureSet(result);
- }());
- return *features;
+const std::optional<FeatureSet>& adb_get_feature_set(std::string* error) {
+ static std::string cached_error [[clang::no_destroy]];
+ static const std::optional<FeatureSet> features
+ [[clang::no_destroy]] ([]() -> std::optional<FeatureSet> {
+ std::string result;
+ if (adb_query(format_host_command("features"), &result, &cached_error)) {
+ return StringToFeatureSet(result);
+ }
+ return std::nullopt;
+ }());
+ if (!features && error) {
+ *error = cached_error;
+ }
+ return features;
}
diff --git a/adb/client/adb_client.h b/adb/client/adb_client.h
index bf0be40..27be28f 100644
--- a/adb/client/adb_client.h
+++ b/adb/client/adb_client.h
@@ -76,7 +76,7 @@
std::string format_host_command(const char* _Nonnull command);
// Get the feature set of the current preferred transport.
-const FeatureSet& adb_get_feature_set();
+const std::optional<FeatureSet>& adb_get_feature_set(std::string* _Nullable error);
#if defined(__linux__)
// Get the path of a file containing the path to the server executable, if the socket spec set via
diff --git a/adb/client/adb_install.cpp b/adb/client/adb_install.cpp
index 3810cc8..59c8563 100644
--- a/adb/client/adb_install.cpp
+++ b/adb/client/adb_install.cpp
@@ -57,11 +57,12 @@
}
static bool can_use_feature(const char* feature) {
- auto&& features = adb_get_feature_set();
- if (features.empty()) {
+ // We ignore errors here, if the device is missing, we'll notice when we try to push install.
+ auto&& features = adb_get_feature_set(nullptr);
+ if (!features) {
return false;
}
- return CanUseFeature(features, feature);
+ return CanUseFeature(*features, feature);
}
static InstallMode best_install_mode() {
diff --git a/adb/client/commandline.cpp b/adb/client/commandline.cpp
index 6a7493f..29f9dc1 100644
--- a/adb/client/commandline.cpp
+++ b/adb/client/commandline.cpp
@@ -672,16 +672,17 @@
}
static int adb_shell(int argc, const char** argv) {
- auto&& features = adb_get_feature_set();
- if (features.empty()) {
- return 1;
+ std::string error;
+ auto&& features = adb_get_feature_set(&error);
+ if (!features) {
+ error_exit("%s", error.c_str());
}
enum PtyAllocationMode { kPtyAuto, kPtyNo, kPtyYes, kPtyDefinitely };
// Defaults.
- char escape_char = '~'; // -e
- bool use_shell_protocol = CanUseFeature(features, kFeatureShell2); // -x
+ char escape_char = '~'; // -e
+ bool use_shell_protocol = CanUseFeature(*features, kFeatureShell2); // -x
PtyAllocationMode tty = use_shell_protocol ? kPtyAuto : kPtyDefinitely; // -t/-T
// Parse shell-specific command-line options.
@@ -757,7 +758,7 @@
if (!use_shell_protocol) {
if (shell_type_arg != kShellServiceArgPty) {
fprintf(stderr, "error: %s only supports allocating a pty\n",
- !CanUseFeature(features, kFeatureShell2) ? "device" : "-x");
+ !CanUseFeature(*features, kFeatureShell2) ? "device" : "-x");
return 1;
} else {
// If we're not using the shell protocol, the type argument must be empty.
@@ -777,11 +778,13 @@
}
static int adb_abb(int argc, const char** argv) {
- auto&& features = adb_get_feature_set();
- if (features.empty()) {
+ std::string error;
+ auto&& features = adb_get_feature_set(&error);
+ if (!features) {
+ error_exit("%s", error.c_str());
return 1;
}
- if (!CanUseFeature(features, kFeatureAbb)) {
+ if (!CanUseFeature(*features, kFeatureAbb)) {
error_exit("abb is not supported by the device");
}
@@ -1159,9 +1162,9 @@
// Use shell protocol if it's supported and the caller doesn't explicitly
// disable it.
if (!disable_shell_protocol) {
- auto&& features = adb_get_feature_set();
- if (!features.empty()) {
- use_shell_protocol = CanUseFeature(features, kFeatureShell2);
+ auto&& features = adb_get_feature_set(nullptr);
+ if (features) {
+ use_shell_protocol = CanUseFeature(*features, kFeatureShell2);
} else {
// Device was unreachable.
attempt_connection = false;
@@ -1810,12 +1813,13 @@
}
return adb_connect_command(android::base::StringPrintf("tcpip:%d", port));
} else if (!strcmp(argv[0], "remount")) {
- auto&& features = adb_get_feature_set();
- if (features.empty()) {
- return 1;
+ std::string error;
+ auto&& features = adb_get_feature_set(&error);
+ if (!features) {
+ error_exit("%s", error.c_str());
}
- if (CanUseFeature(features, kFeatureRemountShell)) {
+ if (CanUseFeature(*features, kFeatureRemountShell)) {
std::vector<const char*> args = {"shell"};
args.insert(args.cend(), argv, argv + argc);
return adb_shell_noinput(args.size(), args.data());
@@ -2034,11 +2038,12 @@
} else if (!strcmp(argv[0], "track-jdwp")) {
return adb_connect_command("track-jdwp");
} else if (!strcmp(argv[0], "track-app")) {
- auto&& features = adb_get_feature_set();
- if (features.empty()) {
- return 1;
+ std::string error;
+ auto&& features = adb_get_feature_set(&error);
+ if (!features) {
+ error_exit("%s", error.c_str());
}
- if (!CanUseFeature(features, kFeatureTrackApp)) {
+ if (!CanUseFeature(*features, kFeatureTrackApp)) {
error_exit("track-app is not supported by the device");
}
TrackAppStreamsCallback callback;
@@ -2064,13 +2069,14 @@
return 0;
} else if (!strcmp(argv[0], "features")) {
// Only list the features common to both the adb client and the device.
- auto&& features = adb_get_feature_set();
- if (features.empty()) {
- return 1;
+ std::string error;
+ auto&& features = adb_get_feature_set(&error);
+ if (!features) {
+ error_exit("%s", error.c_str());
}
- for (const std::string& name : features) {
- if (CanUseFeature(features, name)) {
+ for (const std::string& name : *features) {
+ if (CanUseFeature(*features, name)) {
printf("%s\n", name.c_str());
}
}
diff --git a/adb/client/file_sync_client.cpp b/adb/client/file_sync_client.cpp
index f2c673a..7185939 100644
--- a/adb/client/file_sync_client.cpp
+++ b/adb/client/file_sync_client.cpp
@@ -225,21 +225,22 @@
class SyncConnection {
public:
- SyncConnection()
- : acknowledgement_buffer_(sizeof(sync_status) + SYNC_DATA_MAX),
- features_(adb_get_feature_set()) {
+ SyncConnection() : acknowledgement_buffer_(sizeof(sync_status) + SYNC_DATA_MAX) {
acknowledgement_buffer_.resize(0);
max = SYNC_DATA_MAX; // TODO: decide at runtime.
- if (features_.empty()) {
- Error("failed to get feature set");
+ std::string error;
+ auto&& features = adb_get_feature_set(&error);
+ if (!features) {
+ Error("failed to get feature set: %s", error.c_str());
} else {
- have_stat_v2_ = CanUseFeature(features_, kFeatureStat2);
- have_ls_v2_ = CanUseFeature(features_, kFeatureLs2);
- have_sendrecv_v2_ = CanUseFeature(features_, kFeatureSendRecv2);
- have_sendrecv_v2_brotli_ = CanUseFeature(features_, kFeatureSendRecv2Brotli);
- have_sendrecv_v2_lz4_ = CanUseFeature(features_, kFeatureSendRecv2LZ4);
- have_sendrecv_v2_dry_run_send_ = CanUseFeature(features_, kFeatureSendRecv2DryRunSend);
+ features_ = &*features;
+ have_stat_v2_ = CanUseFeature(*features, kFeatureStat2);
+ have_ls_v2_ = CanUseFeature(*features, kFeatureLs2);
+ have_sendrecv_v2_ = CanUseFeature(*features, kFeatureSendRecv2);
+ have_sendrecv_v2_brotli_ = CanUseFeature(*features, kFeatureSendRecv2Brotli);
+ have_sendrecv_v2_lz4_ = CanUseFeature(*features, kFeatureSendRecv2LZ4);
+ have_sendrecv_v2_dry_run_send_ = CanUseFeature(*features, kFeatureSendRecv2DryRunSend);
std::string error;
fd.reset(adb_connect("sync:", &error));
if (fd < 0) {
@@ -283,7 +284,7 @@
return compression;
}
- const FeatureSet& Features() const { return features_; }
+ const FeatureSet& Features() const { return *features_; }
bool IsValid() { return fd >= 0; }
@@ -921,7 +922,7 @@
private:
std::deque<std::pair<std::string, std::string>> deferred_acknowledgements_;
Block acknowledgement_buffer_;
- const FeatureSet& features_;
+ const FeatureSet* features_ = nullptr;
bool have_stat_v2_;
bool have_ls_v2_;
bool have_sendrecv_v2_;