am af60acef: am 98714882: Merge "adb start-server: Use a separate fd for sending initial OK"
* commit 'af60acef8231f03f7c736ed8ce86bd59979f9f6c':
adb start-server: Use a separate fd for sending initial OK
diff --git a/adb/adb.cpp b/adb/adb.cpp
index fd46dea..dd6c555 100644
--- a/adb/adb.cpp
+++ b/adb/adb.cpp
@@ -698,7 +698,7 @@
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 @@
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 309b0e9..b0e53f0 100644
--- a/adb/adb.h
+++ b/adb/adb.h
@@ -299,7 +299,7 @@
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 6b48621..af3a6a1 100644
--- a/adb/client/main.cpp
+++ b/adb/client/main.cpp
@@ -132,7 +132,7 @@
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 @@
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
- android::base::WriteStringToFd("OK\n", reply_fd);
+ // 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
close_stdin();
setup_daemon_logging();
}
@@ -204,7 +195,6 @@
adb_sysdeps_init();
adb_trace_init(argv);
- D("Handling commandline()\n");
return adb_commandline(argc - 1, const_cast<const char**>(argv + 1));
}
diff --git a/adb/commandline.cpp b/adb/commandline.cpp
index e0e608e..5665e12 100644
--- a/adb/commandline.cpp
+++ b/adb/commandline.cpp
@@ -962,6 +962,14 @@
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
@@ -979,7 +987,7 @@
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",
@@ -997,6 +1005,16 @@
} 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') {
@@ -1074,7 +1092,11 @@
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);
}