adb: Fix PTY logic for non-interactive shells.
Change `adb shell` so that interactive sessions use a PTY but
non-interactive do not. This matches `ssh` functionality better
and also enables future work to split stdout/stderr for
non-interactive sessions.
A test to verify this behavior is added to test_device.py with
supporting modifications in device.py.
Bug: http://b/21215503
Change-Id: Ib4ba40df85f82ddef4e0dd557952271c859d1c7b
diff --git a/services.cpp b/services.cpp
index 68545d8..255be09 100644
--- a/services.cpp
+++ b/services.cpp
@@ -59,6 +59,10 @@
void *cookie;
};
+enum class SubprocessType {
+ kPty,
+ kRaw,
+};
void *service_bootstrap_func(void *x)
{
@@ -389,17 +393,27 @@
}
}
-static int create_subproc_thread(const char *name, bool pty = false) {
+// Starts a subprocess and spawns a thread to wait for the subprocess to finish
+// and trigger the necessary cleanup.
+//
+// |name| is the command to execute in the subprocess; empty string will start
+// an interactive session.
+// |type| selects between a PTY or raw subprocess.
+//
+// Returns an open file descriptor tied to the subprocess stdin/stdout/stderr.
+static int create_subproc_thread(const char *name, SubprocessType type) {
const char *arg0, *arg1;
- if (name == 0 || *name == 0) {
- arg0 = "-"; arg1 = 0;
+ if (*name == '\0') {
+ arg0 = "-";
+ arg1 = nullptr;
} else {
- arg0 = "-c"; arg1 = name;
+ arg0 = "-c";
+ arg1 = name;
}
pid_t pid = -1;
int ret_fd;
- if (pty) {
+ if (type == SubprocessType::kPty) {
ret_fd = create_subproc_pty(SHELL_COMMAND, arg0, arg1, &pid);
} else {
ret_fd = create_subproc_raw(SHELL_COMMAND, arg0, arg1, &pid);
@@ -466,9 +480,16 @@
} else if (!strncmp(name, "jdwp:", 5)) {
ret = create_jdwp_connection_fd(atoi(name+5));
} else if(!strncmp(name, "shell:", 6)) {
- ret = create_subproc_thread(name + 6, true);
+ const char* args = name + 6;
+ if (*args) {
+ // Non-interactive session uses a raw subprocess.
+ ret = create_subproc_thread(args, SubprocessType::kRaw);
+ } else {
+ // Interactive session uses a PTY subprocess.
+ ret = create_subproc_thread(args, SubprocessType::kPty);
+ }
} else if(!strncmp(name, "exec:", 5)) {
- ret = create_subproc_thread(name + 5);
+ ret = create_subproc_thread(name + 5, SubprocessType::kRaw);
} else if(!strncmp(name, "sync:", 5)) {
ret = create_service_thread(file_sync_service, NULL);
} else if(!strncmp(name, "remount:", 8)) {
@@ -482,10 +503,13 @@
} else if(!strncmp(name, "unroot:", 7)) {
ret = create_service_thread(restart_unroot_service, NULL);
} else if(!strncmp(name, "backup:", 7)) {
- ret = create_subproc_thread(android::base::StringPrintf("/system/bin/bu backup %s",
- (name + 7)).c_str());
+ ret = create_subproc_thread(
+ android::base::StringPrintf("/system/bin/bu backup %s",
+ (name + 7)).c_str(),
+ SubprocessType::kRaw);
} else if(!strncmp(name, "restore:", 8)) {
- ret = create_subproc_thread("/system/bin/bu restore");
+ ret = create_subproc_thread("/system/bin/bu restore",
+ SubprocessType::kRaw);
} else if(!strncmp(name, "tcpip:", 6)) {
int port;
if (sscanf(name + 6, "%d", &port) != 1) {