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

1 files changed, 34 insertions, 10 deletions

diff --git a/services.cpp b/services.cpp
index 68545d8..255be09 100644
--- a/services.cpp
+++ b/services.cpp
@@ -59,6 +59,10 @@ struct stinfo {
     void *cookie;
 };
 
+enum class SubprocessType {
+    kPty,
+    kRaw,
+};
 
 void *service_bootstrap_func(void *x)
 {
@@ -389,17 +393,27 @@ static void subproc_waiter_service(int fd, void *cookie)
     }
 }
 
-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 @@ int service_to_fd(const char *name)
     } 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 @@ int service_to_fd(const char *name)
     } 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) {