Merge "adb: start-server and kill-server error output"

adb/adb.cpp

@@ -888,6 +888,12 @@ int handle_host_request(const char* service, TransportType type,
         fprintf(stderr, "adb server killed by remote request\n");
         fflush(stdout);
         SendOkay(reply_fd);
+
+        // At least on Windows, if we exit() without shutdown(SD_SEND) or
+        // closesocket(), the client's next recv() will error-out with
+        // WSAECONNRESET and they'll never read the OKAY.
+        adb_shutdown(reply_fd);
+
         exit(0);
     }
 

adb/adb_client.cpp

@@ -197,6 +197,7 @@ int adb_connect(const std::string& service, std::string* error) {
     D("adb_connect: service %s\n", service.c_str());
     if (fd == -2 && __adb_server_name) {
         fprintf(stderr,"** Cannot start server on remote host\n");
+        // error is the original network connection error
         return fd;
     } else if (fd == -2) {
         fprintf(stdout,"* daemon not running. starting it now on port %d *\n",
@@ -204,6 +205,10 @@ int adb_connect(const std::string& service, std::string* error) {
     start_server:
         if (launch_server(__adb_server_port)) {
             fprintf(stderr,"* failed to start daemon *\n");
+            // launch_server() has already printed detailed error info, so just
+            // return a generic error string about the overall adb_connect()
+            // that the caller requested.
+            *error = "cannot connect to daemon";
             return -1;
         } else {
             fprintf(stdout,"* daemon started successfully *\n");
@@ -225,7 +230,10 @@ int adb_connect(const std::string& service, std::string* error) {
             adb_close(fd);
 
             if (sscanf(&version_string[0], "%04x", &version) != 1) {
-                goto error;
+                *error = android::base::StringPrintf(
+                        "cannot parse version string: %s",
+                        version_string.c_str());
+                return -1;
             }
         } else {
             // if fd is -1, then check for "unknown host service",
@@ -239,7 +247,13 @@ int adb_connect(const std::string& service, std::string* error) {
         if (version != ADB_SERVER_VERSION) {
             printf("adb server is out of date.  killing...\n");
             fd = _adb_connect("host:kill", error);
-            adb_close(fd);
+            if (fd >= 0) {
+                adb_close(fd);
+            } else {
+                // If we couldn't connect to the server or had some other error,
+                // report it, but still try to start the server.
+                fprintf(stderr, "error: %s\n", error->c_str());
+            }
 
             /* XXX can we better detect its death? */
             adb_sleep_ms(2000);

adb/client/main.cpp

@@ -178,7 +178,9 @@ int adb_main(int is_daemon, int server_port, int ack_reply_fd) {
 #else
         // TODO(danalbert): Can't use SendOkay because we're sending "OK\n", not
         // "OKAY".
-        android::base::WriteStringToFd("OK\n", ack_reply_fd);
+        if (!android::base::WriteStringToFd("OK\n", ack_reply_fd)) {
+            fatal_errno("error writing ACK to fd %d", ack_reply_fd);
+        }
         unix_close(ack_reply_fd);
 #endif
         close_stdin();

adb/commandline.cpp

@@ -968,7 +968,7 @@ int adb_commandline(int argc, const char **argv) {
         server_port = strtol(server_port_str, nullptr, 0);
         if (server_port <= 0 || server_port > 65535) {
             fprintf(stderr,
-                    "adb: Env var ANDROID_ADB_SERVER_PORT must be a positive number less than 65535. Got \"%s\"\n",
+                    "adb: Env var ANDROID_ADB_SERVER_PORT must be a positive number less than 65536. Got \"%s\"\n",
                     server_port_str);
             return usage();
         }
@@ -1233,11 +1233,22 @@ int adb_commandline(int argc, const char **argv) {
     else if (!strcmp(argv[0], "kill-server")) {
         std::string error;
         int fd = _adb_connect("host:kill", &error);
-        if (fd == -1) {
+        if (fd == -2) {
+            // Failed to make network connection to server. Don't output the
+            // network error since that is expected.
             fprintf(stderr,"* server not running *\n");
+            // Successful exit code because the server is already "killed".
+            return 0;
+        } else if (fd == -1) {
+            // Some other error.
+            fprintf(stderr, "error: %s\n", error.c_str());
             return 1;
+        } else {
+            // Successfully connected, kill command sent, okay status came back.
+            // Server should exit() in a moment, if not already.
+            adb_close(fd);
+            return 0;
         }
-        return 0;
     }
     else if (!strcmp(argv[0], "sideload")) {
         if (argc != 2) return usage();
@@ -1421,7 +1432,11 @@ int adb_commandline(int argc, const char **argv) {
     }
     else if (!strcmp(argv[0], "start-server")) {
         std::string error;
-        return adb_connect("host:start-server", &error);
+        const int result = adb_connect("host:start-server", &error);
+        if (result < 0) {
+            fprintf(stderr, "error: %s\n", error.c_str());
+        }
+        return result;
     }
     else if (!strcmp(argv[0], "backup")) {
         return backup(argc, argv);