diff --git a/adb/adb.cpp b/adb/adb.cpp index fd46deac1..dd6c55511 100644 --- a/adb/adb.cpp +++ b/adb/adb.cpp @@ -698,7 +698,7 @@ int launch_server(int server_port) int fd[2]; // set up a pipe so the child can tell us when it is ready. - // fd[0] will be parent's end, and fd[1] will get mapped to stderr in the child. + // fd[0] will be parent's end, and the child will write on fd[1] if (pipe(fd)) { fprintf(stderr, "pipe failed in launch_server, errno: %d\n", errno); return -1; @@ -710,16 +710,14 @@ int launch_server(int server_port) if (pid == 0) { // child side of the fork - // redirect stderr to the pipe - // we use stderr instead of stdout due to stdout's buffering behavior. adb_close(fd[0]); - dup2(fd[1], STDERR_FILENO); - adb_close(fd[1]); char str_port[30]; - snprintf(str_port, sizeof(str_port), "%d", server_port); + snprintf(str_port, sizeof(str_port), "%d", server_port); + char reply_fd[30]; + snprintf(reply_fd, sizeof(reply_fd), "%d", fd[1]); // child process - int result = execl(path, "adb", "-P", str_port, "fork-server", "server", NULL); + int result = execl(path, "adb", "-P", str_port, "fork-server", "server", "--reply-fd", reply_fd, NULL); // this should not return fprintf(stderr, "OOPS! execl returned %d, errno: %d\n", result, errno); } else { diff --git a/adb/adb.h b/adb/adb.h index 309b0e9bc..b0e53f026 100644 --- a/adb/adb.h +++ b/adb/adb.h @@ -299,7 +299,7 @@ void handle_packet(apacket *p, atransport *t); void get_my_path(char *s, size_t maxLen); int launch_server(int server_port); -int adb_main(int is_daemon, int server_port); +int adb_main(int is_daemon, int server_port, int ack_reply_fd); /* initialize a transport object's func pointers and state */ #if ADB_HOST diff --git a/adb/client/main.cpp b/adb/client/main.cpp index 6b4862138..af3a6a108 100644 --- a/adb/client/main.cpp +++ b/adb/client/main.cpp @@ -132,7 +132,7 @@ static void setup_daemon_logging(void) { fprintf(stderr, "--- adb starting (pid %d) ---\n", getpid()); } -int adb_main(int is_daemon, int server_port) { +int adb_main(int is_daemon, int server_port, int ack_reply_fd) { HOST = 1; #if defined(_WIN32) @@ -157,29 +157,20 @@ int adb_main(int is_daemon, int server_port) { LOG(FATAL) << "Could not install *smartsocket* listener: " << error; } + // Inform our parent that we are up and running. if (is_daemon) { - // Inform our parent that we are up and running. - // TODO(danalbert): Can't use SendOkay because we're sending "OK\n", not - // "OKAY". - // TODO(danalbert): Why do we use stdout for Windows? There is a - // comment in launch_server() that suggests that non-Windows uses - // stderr because it is non-buffered. So perhaps the history is that - // stdout was preferred for all platforms, but it was discovered that - // non-Windows needed a non-buffered fd, so stderr was used there. - // Note that using stderr on unix means that if you do - // `ADB_TRACE=all adb start-server`, it will say "ADB server didn't ACK" - // and "* failed to start daemon *" because the adb server will write - // logging to stderr, obscuring the OK\n output that is sent to stderr. #if defined(_WIN32) - int reply_fd = STDOUT_FILENO; // Change stdout mode to binary so \n => \r\n translation does not // occur. In a moment stdout will be reopened to the daemon log file // anyway. - _setmode(reply_fd, _O_BINARY); -#else - int reply_fd = STDERR_FILENO; + _setmode(ack_reply_fd, _O_BINARY); +#endif + // TODO(danalbert): Can't use SendOkay because we're sending "OK\n", not + // "OKAY". + android::base::WriteStringToFd("OK\n", ack_reply_fd); +#if !defined(_WIN32) + adb_close(ack_reply_fd); #endif - android::base::WriteStringToFd("OK\n", reply_fd); close_stdin(); setup_daemon_logging(); } @@ -204,7 +195,6 @@ int main(int argc, char** argv) { adb_sysdeps_init(); adb_trace_init(argv); - D("Handling commandline()\n"); return adb_commandline(argc - 1, const_cast(argv + 1)); } diff --git a/adb/commandline.cpp b/adb/commandline.cpp index d7a0c8d5b..2e182e898 100644 --- a/adb/commandline.cpp +++ b/adb/commandline.cpp @@ -954,6 +954,14 @@ int adb_commandline(int argc, const char **argv) { int r; TransportType transport_type = kTransportAny; +#if defined(_WIN32) + // TODO(compareandswap): Windows should use a separate reply fd too. + int ack_reply_fd = STDOUT_FILENO; +#else + int ack_reply_fd = -1; +#endif + + // If defined, this should be an absolute path to // the directory containing all of the various system images // for a particular product. If not defined, and the adb @@ -971,7 +979,7 @@ int adb_commandline(int argc, const char **argv) { const char* server_port_str = getenv("ANDROID_ADB_SERVER_PORT"); int server_port = DEFAULT_ADB_PORT; if (server_port_str && strlen(server_port_str) > 0) { - server_port = (int) strtol(server_port_str, NULL, 0); + 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", @@ -989,6 +997,16 @@ int adb_commandline(int argc, const char **argv) { } else if (!strcmp(argv[0], "fork-server")) { /* this is a special flag used only when the ADB client launches the ADB Server */ is_daemon = 1; + } else if (!strcmp(argv[0], "--reply-fd")) { + if (argc < 2) return usage(); + const char* reply_fd_str = argv[1]; + argc--; + argv++; + ack_reply_fd = strtol(reply_fd_str, nullptr, 10); + if (ack_reply_fd <= 2) { // Disallow stdin, stdout, and stderr. + fprintf(stderr, "adb: invalid reply fd \"%s\"\n", reply_fd_str); + return usage(); + } } else if (!strncmp(argv[0], "-p", 2)) { const char* product = nullptr; if (argv[0][2] == '\0') { @@ -1066,7 +1084,11 @@ int adb_commandline(int argc, const char **argv) { if (is_server) { if (no_daemon || is_daemon) { - r = adb_main(is_daemon, server_port); + if (ack_reply_fd == -1) { + fprintf(stderr, "reply fd for adb server to client communication not specified.\n"); + return usage(); + } + r = adb_main(is_daemon, server_port, ack_reply_fd); } else { r = launch_server(server_port); }