adb: win32: write ACK to separate pipe instead of stdout
The win32 version of 9f2d1a9cfc04e1d5970823da1878097288a9a9cd. The big
technique is to fit a Win32 HANDLE value in an int because it only uses
32-bits. This allows most of the other adb code to stay the same.
Also, fix a regression in the 'adb server nodaemon' command that was
erroneously returning an error when --reply-fd was not used, which
should not be necessary for this particular command.
Change-Id: I37e9c609014b813af93bf0d6c12f665b59c93c41
Signed-off-by: Spencer Low <CompareAndSwap@gmail.com>
diff --git a/adb/adb.cpp b/adb/adb.cpp
index dd6c555..821b785 100644
--- a/adb/adb.cpp
+++ b/adb/adb.cpp
@@ -630,7 +630,7 @@
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 @@
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,
diff --git a/adb/client/main.cpp b/adb/client/main.cpp
index af3a6a1..73acbb0 100644
--- a/adb/client/main.cpp
+++ b/adb/client/main.cpp
@@ -160,16 +160,24 @@
// 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
- // occur. In a moment stdout will be reopened to the daemon log file
- // anyway.
- _setmode(ack_reply_fd, _O_BINARY);
-#endif
+ const HANDLE ack_reply_handle = cast_int_to_handle(ack_reply_fd);
+ const CHAR ack[] = "OK\n";
+ const DWORD bytes_to_write = arraysize(ack) - 1;
+ DWORD written = 0;
+ 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)
- adb_close(ack_reply_fd);
+ unix_close(ack_reply_fd);
#endif
close_stdin();
setup_daemon_logging();
diff --git a/adb/commandline.cpp b/adb/commandline.cpp
index 2e182e8..1e1690e 100644
--- a/adb/commandline.cpp
+++ b/adb/commandline.cpp
@@ -953,14 +953,7 @@
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
@@ -1003,7 +996,14 @@
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();
}
@@ -1084,7 +1084,7 @@
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();
}
diff --git a/adb/sysdeps.h b/adb/sysdeps.h
index 9189955..6f3c443 100644
--- a/adb/sysdeps.h
+++ b/adb/sysdeps.h
@@ -338,6 +338,23 @@
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"
