am 61ff977b: am 1f36ae49: Merge "adb: win32: write ACK to separate pipe instead of stdout"

* commit '61ff977ba49f43d87fed86eb5b2e2bfc279f902b':
  adb: win32: write ACK to separate pipe instead of stdout

adb/adb.cpp

@@ -630,7 +630,7 @@ int launch_server(int server_port)
     ZeroMemory( &startup, sizeof(startup) );
     startup.cb = sizeof(startup);
     startup.hStdInput  = nul_read;
-    startup.hStdOutput = pipe_write;
+    startup.hStdOutput = nul_write;
     startup.hStdError  = nul_write;
     startup.dwFlags    = STARTF_USESTDHANDLES;
 
@@ -645,9 +645,23 @@ int launch_server(int server_port)
                 SystemErrorCodeToString(GetLastError()).c_str());
         return -1;
     }
+
+    // Verify that the pipe_write handle value can be passed on the command line
+    // as %d and that the rest of adb code can pass it around in an int.
+    const int pipe_write_as_int = cast_handle_to_int(pipe_write);
+    if (cast_int_to_handle(pipe_write_as_int) != pipe_write) {
+        // If this fires, either handle values are larger than 32-bits or else
+        // there is a bug in our casting.
+        // https://msdn.microsoft.com/en-us/library/windows/desktop/aa384203%28v=vs.85%29.aspx
+        fprintf(stderr, "CreatePipe handle value too large: 0x%p\n",
+                pipe_write);
+        return -1;
+    }
+
     WCHAR args[64];
     snwprintf(args, arraysize(args),
-              L"adb -P %d fork-server server", server_port);
+              L"adb -P %d fork-server server --reply-fd %d", server_port,
+              pipe_write_as_int);
     ret = CreateProcessW(
             program_path,                              /* program path  */
             args,

adb/client/main.cpp

@@ -160,16 +160,24 @@ int adb_main(int is_daemon, int server_port, int ack_reply_fd) {
     // Inform our parent that we are up and running.
     if (is_daemon) {
 #if defined(_WIN32)
-        // Change stdout mode to binary so \n => \r\n translation does not
+        const HANDLE ack_reply_handle = cast_int_to_handle(ack_reply_fd);
-        // occur. In a moment stdout will be reopened to the daemon log file
+        const CHAR ack[] = "OK\n";
-        // anyway.
+        const DWORD bytes_to_write = arraysize(ack) - 1;
-        _setmode(ack_reply_fd, _O_BINARY);
+        DWORD written = 0;
-#endif
+        if (!WriteFile(ack_reply_handle, ack, bytes_to_write, &written, NULL)) {
+            fatal("adb: cannot write ACK to handle 0x%p: %s", ack_reply_handle,
+                  SystemErrorCodeToString(GetLastError()).c_str());
+        }
+        if (written != bytes_to_write) {
+            fatal("adb: cannot write %lu bytes of ACK: only wrote %lu bytes",
+                  bytes_to_write, written);
+        }
+        CloseHandle(ack_reply_handle);
+#else
         // 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)
+        unix_close(ack_reply_fd);
-        adb_close(ack_reply_fd);
 #endif
         close_stdin();
         setup_daemon_logging();

adb/commandline.cpp

@@ -961,14 +961,7 @@ int adb_commandline(int argc, const char **argv) {
     int is_server = 0;
     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
@@ -1011,7 +1004,14 @@ int adb_commandline(int argc, const char **argv) {
             argc--;
             argv++;
             ack_reply_fd = strtol(reply_fd_str, nullptr, 10);
+#ifdef _WIN32
+            const HANDLE ack_reply_handle = cast_int_to_handle(ack_reply_fd);
+            if ((GetStdHandle(STD_INPUT_HANDLE) == ack_reply_handle) ||
+                (GetStdHandle(STD_OUTPUT_HANDLE) == ack_reply_handle) ||
+                (GetStdHandle(STD_ERROR_HANDLE) == ack_reply_handle)) {
+#else
             if (ack_reply_fd <= 2) { // Disallow stdin, stdout, and stderr.
+#endif
                 fprintf(stderr, "adb: invalid reply fd \"%s\"\n", reply_fd_str);
                 return usage();
             }
@@ -1092,7 +1092,7 @@ int adb_commandline(int argc, const char **argv) {
 
     if (is_server) {
         if (no_daemon || is_daemon) {
-            if (ack_reply_fd == -1) {
+            if (is_daemon && (ack_reply_fd == -1)) {
                 fprintf(stderr, "reply fd for adb server to client communication not specified.\n");
                 return usage();
             }

adb/sysdeps.h

@@ -338,6 +338,23 @@ private:
     char** narrow_args;
 };
 
+// Windows HANDLE values only use 32-bits of the type, even on 64-bit machines,
+// so they can fit in an int. To convert back, we just need to sign-extend.
+// https://msdn.microsoft.com/en-us/library/windows/desktop/aa384203%28v=vs.85%29.aspx
+// Note that this does not make a HANDLE value work with APIs like open(), nor
+// does this make a value from open() passable to APIs taking a HANDLE. This
+// just lets you take a HANDLE, pass it around as an int, and then use it again
+// as a HANDLE.
+inline int cast_handle_to_int(const HANDLE h) {
+    // truncate
+    return static_cast<int>(reinterpret_cast<INT_PTR>(h));
+}
+
+inline HANDLE cast_int_to_handle(const int fd) {
+    // sign-extend
+    return reinterpret_cast<HANDLE>(static_cast<INT_PTR>(fd));
+}
+
 #else /* !_WIN32 a.k.a. Unix */
 
 #include "fdevent.h"