adb: start-server and kill-server error output
- handle_host_request
- When the host:kill command comes in, shutdown the socket before
calling exit(). If we don't do this, the client will output error info
even though everything is working ok.
- adb_connect()
- If we can't parse the version string, explain this in error output
and don't goto error which would try to close an fd we already closed.
- If host:kill doesn't work, output error info. Don't try to close
already closed fd.
- adb_main()
- If writing the ACK somehow has an error, output error info (I doubt
this will ever get hit).
- adb_commandline()
- Fix typo about max port number.
- Make 'adb kill-server' and 'adb start-server' output any detailed
error info.
Change-Id: Id1a309cc1bf516f7f49bd332b34d30f148b406da
Signed-off-by: Spencer Low <CompareAndSwap@gmail.com>
diff --git a/adb/adb.cpp b/adb/adb.cpp
index 548ca54..b83d164 100644
--- a/adb/adb.cpp
+++ b/adb/adb.cpp
@@ -888,6 +888,12 @@
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);
}
diff --git a/adb/adb_client.cpp b/adb/adb_client.cpp
index afff2ef..4bf647c 100644
--- a/adb/adb_client.cpp
+++ b/adb/adb_client.cpp
@@ -197,6 +197,7 @@
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 @@
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 @@
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 @@
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);
diff --git a/adb/client/main.cpp b/adb/client/main.cpp
index fca1762..cd06f4b 100644
--- a/adb/client/main.cpp
+++ b/adb/client/main.cpp
@@ -178,7 +178,9 @@
#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();
diff --git a/adb/commandline.cpp b/adb/commandline.cpp
index 0ac3556..1c5b5bd 100644
--- a/adb/commandline.cpp
+++ b/adb/commandline.cpp
@@ -968,7 +968,7 @@
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 @@
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 @@
}
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);