diff --git a/adb/adb.cpp b/adb/adb.cpp index dd6c55511..821b785df 100644 --- a/adb/adb.cpp +++ b/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, diff --git a/adb/client/main.cpp b/adb/client/main.cpp index af3a6a108..73acbb0b8 100644 --- a/adb/client/main.cpp +++ b/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 - // 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 2e182e898..1e1690edc 100644 --- a/adb/commandline.cpp +++ b/adb/commandline.cpp @@ -953,14 +953,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 @@ -1003,7 +996,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(); } @@ -1084,7 +1084,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(); } diff --git a/adb/sysdeps.h b/adb/sysdeps.h index 918995536..6f3c44302 100644 --- a/adb/sysdeps.h +++ b/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(reinterpret_cast(h)); +} + +inline HANDLE cast_int_to_handle(const int fd) { + // sign-extend + return reinterpret_cast(static_cast(fd)); +} + #else /* !_WIN32 a.k.a. Unix */ #include "fdevent.h"