From 7f939285dea16391721f79a446311b8830e43acd Mon Sep 17 00:00:00 2001 From: Mark Mentovai Date: Mon, 2 Nov 2015 17:00:06 -0500 Subject: [PATCH] win: Rename CrashpadClient::SetHandler() to SetHandlerIPCPipe() In https://codereview.chromium.org/1414533006/, I'm adding a few Mac-specific SetHandler() variants, so it makes sense to name each SetHandler() variant for what it does. I'm also making it take a wstring argument, which seems like a more natural fit for what it does. There should be fewer string conversions this way. R=scottmg@chromium.org Review URL: https://codereview.chromium.org/1406993008 . --- client/crashpad_client.h | 12 ++++----- client/crashpad_client_win.cc | 25 +++++++++---------- handler/handler.gyp | 3 --- handler/win/crashy_test_program.cc | 13 +++++----- handler/win/crashy_test_z7_loader.cc | 9 +++---- handler/win/self_destroying_test_program.cc | 9 +++---- .../crashpad_snapshot_test_crashing_child.cc | 9 +++++-- ...pad_snapshot_test_dump_without_crashing.cc | 8 ++++-- util/win/exception_handler_server_test.cc | 7 +++--- 9 files changed, 48 insertions(+), 47 deletions(-) diff --git a/client/crashpad_client.h b/client/crashpad_client.h index c45f0607..d3d7e347 100644 --- a/client/crashpad_client.h +++ b/client/crashpad_client.h @@ -51,9 +51,6 @@ class CrashpadClient { //! send right corresponding to a receive right held by the handler process. //! The handler process runs an exception server on this port. //! - //! On Windows, SetHandler() is normally used instead since the handler is - //! started by other means. - //! //! \param[in] handler The path to a Crashpad handler executable. //! \param[in] database The path to a Crashpad database. The handler will be //! started with this path as its `--database` argument. @@ -85,10 +82,11 @@ class CrashpadClient { //! registration is done with a crash handler using the appropriate database //! and upload server. //! - //! \param[in] ipc_port The full name of the crash handler IPC port. + //! \param[in] ipc_pipe The full name of the crash handler IPC pipe. This is + //! a string of the form `"\\.\pipe\NAME"`. //! //! \return `true` on success and `false` on failure. - bool SetHandler(const std::string& ipc_port); + bool SetHandlerIPCPipe(const std::wstring& ipc_pipe); //! \brief Requests that the handler capture a dump even though there hasn't //! been a crash. @@ -101,7 +99,7 @@ class CrashpadClient { //! \brief Configures the process to direct its crashes to a Crashpad handler. //! //! The Crashpad handler must previously have been started by StartHandler() - //! or configured by SetHandler(). + //! or configured by SetHandlerIPCPipe(). //! //! On Mac OS X, this method sets the task’s exception port for `EXC_CRASH`, //! `EXC_RESOURCE`, and `EXC_GUARD` exceptions to the Mach send right obtained @@ -147,7 +145,7 @@ class CrashpadClient { #if defined(OS_MACOSX) base::mac::ScopedMachSendRight exception_port_; #elif defined(OS_WIN) - std::string ipc_port_; + std::wstring ipc_pipe_; #endif DISALLOW_COPY_AND_ASSIGN(CrashpadClient); diff --git a/client/crashpad_client_win.cc b/client/crashpad_client_win.cc index 5e7346fa..c4715ad5 100644 --- a/client/crashpad_client_win.cc +++ b/client/crashpad_client_win.cc @@ -115,7 +115,7 @@ std::wstring FormatArgumentString(const std::string& name, namespace crashpad { CrashpadClient::CrashpadClient() - : ipc_port_() { + : ipc_pipe_() { } CrashpadClient::~CrashpadClient() { @@ -127,13 +127,14 @@ bool CrashpadClient::StartHandler( const std::string& url, const std::map& annotations, const std::vector& arguments) { - DCHECK(ipc_port_.empty()); + DCHECK(ipc_pipe_.empty()); - ipc_port_ = + std::string ipc_pipe = base::StringPrintf("\\\\.\\pipe\\crashpad_%d_", GetCurrentProcessId()); for (int index = 0; index < 16; ++index) { - ipc_port_.append(1, static_cast(base::RandInt('A', 'Z'))); + ipc_pipe.append(1, static_cast(base::RandInt('A', 'Z'))); } + ipc_pipe_ = base::UTF8ToUTF16(ipc_pipe); std::wstring command_line; AppendCommandLineArgument(handler.value(), &command_line); @@ -156,8 +157,7 @@ bool CrashpadClient::StartHandler( base::UTF8ToUTF16(kv.first + '=' + kv.second)), &command_line); } - AppendCommandLineArgument(FormatArgumentString("pipe-name", - base::UTF8ToUTF16(ipc_port_)), + AppendCommandLineArgument(FormatArgumentString("pipe-name", ipc_pipe_), &command_line); STARTUPINFO startup_info = {}; @@ -191,17 +191,17 @@ bool CrashpadClient::StartHandler( return true; } -bool CrashpadClient::SetHandler(const std::string& ipc_port) { - DCHECK(ipc_port_.empty()); - DCHECK(!ipc_port.empty()); +bool CrashpadClient::SetHandlerIPCPipe(const std::wstring& ipc_pipe) { + DCHECK(ipc_pipe_.empty()); + DCHECK(!ipc_pipe.empty()); - ipc_port_ = ipc_port; + ipc_pipe_ = ipc_pipe; return true; } bool CrashpadClient::UseHandler() { - DCHECK(!ipc_port_.empty()); + DCHECK(!ipc_pipe_.empty()); DCHECK_EQ(g_signal_exception, INVALID_HANDLE_VALUE); DCHECK_EQ(g_signal_non_crash_dump, INVALID_HANDLE_VALUE); DCHECK_EQ(g_non_crash_dump_done, INVALID_HANDLE_VALUE); @@ -232,8 +232,7 @@ bool CrashpadClient::UseHandler() { ServerToClientMessage response = {0}; - if (!SendToCrashHandlerServer( - base::UTF8ToUTF16(ipc_port_), message, &response)) { + if (!SendToCrashHandlerServer(ipc_pipe_, message, &response)) { return false; } diff --git a/handler/handler.gyp b/handler/handler.gyp index 398c5404..20df3d73 100644 --- a/handler/handler.gyp +++ b/handler/handler.gyp @@ -81,7 +81,6 @@ 'dependencies': [ '../client/client.gyp:crashpad_client', '../third_party/mini_chromium/mini_chromium.gyp:base', - '../tools/tools.gyp:crashpad_tool_support', '../util/util.gyp:crashpad_util', ], 'include_dirs': [ @@ -99,7 +98,6 @@ '../compat/compat.gyp:crashpad_compat', '../snapshot/snapshot.gyp:crashpad_snapshot', '../third_party/mini_chromium/mini_chromium.gyp:base', - '../tools/tools.gyp:crashpad_tool_support', '../util/util.gyp:crashpad_util', ], 'include_dirs': [ @@ -121,7 +119,6 @@ '../client/client.gyp:crashpad_client', '../test/test.gyp:crashpad_test', '../third_party/mini_chromium/mini_chromium.gyp:base', - '../tools/tools.gyp:crashpad_tool_support', ], 'include_dirs': [ '..', diff --git a/handler/win/crashy_test_program.cc b/handler/win/crashy_test_program.cc index 15c7b613..7d61c256 100644 --- a/handler/win/crashy_test_program.cc +++ b/handler/win/crashy_test_program.cc @@ -12,6 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. +#include #include #include #include @@ -28,9 +29,7 @@ #include "base/basictypes.h" #include "base/files/file_path.h" #include "base/logging.h" -#include "base/strings/utf_string_conversions.h" #include "client/crashpad_client.h" -#include "tools/tool_support.h" #include "util/win/critical_section_with_debug_info.h" #include "util/win/get_function.h" @@ -94,17 +93,17 @@ void SomeCrashyFunction() { *foo = 42; } -int CrashyMain(int argc, char* argv[]) { +int CrashyMain(int argc, wchar_t* argv[]) { CrashpadClient client; if (argc == 2) { - if (!client.SetHandler(argv[1])) { + if (!client.SetHandlerIPCPipe(argv[1])) { LOG(ERROR) << "SetHandler"; return EXIT_FAILURE; } } else if (argc == 3) { - if (!client.StartHandler(base::FilePath(base::UTF8ToUTF16(argv[1])), - base::FilePath(base::UTF8ToUTF16(argv[2])), + if (!client.StartHandler(base::FilePath(argv[1]), + base::FilePath(argv[2]), std::string(), std::map(), std::vector())) { @@ -138,5 +137,5 @@ int CrashyMain(int argc, char* argv[]) { } // namespace crashpad int wmain(int argc, wchar_t* argv[]) { - return crashpad::ToolSupport::Wmain(argc, argv, crashpad::CrashyMain); + return crashpad::CrashyMain(argc, argv); } diff --git a/handler/win/crashy_test_z7_loader.cc b/handler/win/crashy_test_z7_loader.cc index 4788dbe9..284afa86 100644 --- a/handler/win/crashy_test_z7_loader.cc +++ b/handler/win/crashy_test_z7_loader.cc @@ -20,7 +20,6 @@ #include "build/build_config.h" #include "client/crashpad_client.h" #include "test/paths.h" -#include "tools/tool_support.h" #if !defined(ARCH_CPU_X86) #error This test is only supported on x86. @@ -29,14 +28,14 @@ namespace crashpad { namespace { -int CrashyLoadZ7Main(int argc, char* argv[]) { +int CrashyLoadZ7Main(int argc, wchar_t* argv[]) { if (argc != 2) { - fprintf(stderr, "Usage: %s \n", argv[0]); + fprintf(stderr, "Usage: %ls \n", argv[0]); return EXIT_FAILURE; } CrashpadClient client; - if (!client.SetHandler(argv[1])) { + if (!client.SetHandlerIPCPipe(argv[1])) { LOG(ERROR) << "SetHandler"; return EXIT_FAILURE; } @@ -69,5 +68,5 @@ int CrashyLoadZ7Main(int argc, char* argv[]) { } // namespace crashpad int wmain(int argc, wchar_t* argv[]) { - return crashpad::ToolSupport::Wmain(argc, argv, crashpad::CrashyLoadZ7Main); + return crashpad::CrashyLoadZ7Main(argc, argv); } diff --git a/handler/win/self_destroying_test_program.cc b/handler/win/self_destroying_test_program.cc index d00fa476..ac358ef0 100644 --- a/handler/win/self_destroying_test_program.cc +++ b/handler/win/self_destroying_test_program.cc @@ -21,7 +21,6 @@ #include "base/strings/stringprintf.h" #include "client/crashpad_client.h" #include "snapshot/win/process_reader_win.h" -#include "tools/tool_support.h" namespace crashpad { namespace { @@ -65,14 +64,14 @@ bool FreeOwnStackAndBreak() { return true; } -int SelfDestroyingMain(int argc, char* argv[]) { +int SelfDestroyingMain(int argc, wchar_t* argv[]) { if (argc != 2) { - fprintf(stderr, "Usage: %s \n", argv[0]); + fprintf(stderr, "Usage: %ls \n", argv[0]); return EXIT_FAILURE; } CrashpadClient client; - if (!client.SetHandler(argv[1])) { + if (!client.SetHandlerIPCPipe(argv[1])) { LOG(ERROR) << "SetHandler"; return EXIT_FAILURE; } @@ -93,5 +92,5 @@ int SelfDestroyingMain(int argc, char* argv[]) { } // namespace crashpad int wmain(int argc, wchar_t* argv[]) { - return crashpad::ToolSupport::Wmain(argc, argv, crashpad::SelfDestroyingMain); + return crashpad::SelfDestroyingMain(argc, argv); } diff --git a/snapshot/win/crashpad_snapshot_test_crashing_child.cc b/snapshot/win/crashpad_snapshot_test_crashing_child.cc index 7f4b8e5b..d4023c65 100644 --- a/snapshot/win/crashpad_snapshot_test_crashing_child.cc +++ b/snapshot/win/crashpad_snapshot_test_crashing_child.cc @@ -14,20 +14,25 @@ #include +#include "base/files/file_path.h" #include "base/logging.h" #include "client/crashpad_client.h" #include "util/file/file_io.h" #include "util/win/address_types.h" +namespace { + __declspec(noinline) crashpad::WinVMAddress CurrentAddress() { return reinterpret_cast(_ReturnAddress()); } -int main(int argc, char* argv[]) { +} // namespace + +int wmain(int argc, wchar_t* argv[]) { CHECK_EQ(argc, 2); crashpad::CrashpadClient client; - CHECK(client.SetHandler(argv[1])); + CHECK(client.SetHandlerIPCPipe(argv[1])); CHECK(client.UseHandler()); HANDLE out = GetStdHandle(STD_OUTPUT_HANDLE); diff --git a/snapshot/win/crashpad_snapshot_test_dump_without_crashing.cc b/snapshot/win/crashpad_snapshot_test_dump_without_crashing.cc index 1ecc18ed..cf5ab950 100644 --- a/snapshot/win/crashpad_snapshot_test_dump_without_crashing.cc +++ b/snapshot/win/crashpad_snapshot_test_dump_without_crashing.cc @@ -20,15 +20,19 @@ #include "util/file/file_io.h" #include "util/win/address_types.h" +namespace { + __declspec(noinline) crashpad::WinVMAddress CurrentAddress() { return reinterpret_cast(_ReturnAddress()); } -int main(int argc, char* argv[]) { +} // namespace + +int wmain(int argc, wchar_t* argv[]) { CHECK_EQ(argc, 2); crashpad::CrashpadClient client; - CHECK(client.SetHandler(argv[1])); + CHECK(client.SetHandlerIPCPipe(argv[1])); CHECK(client.UseHandler()); HANDLE out = GetStdHandle(STD_OUTPUT_HANDLE); diff --git a/util/win/exception_handler_server_test.cc b/util/win/exception_handler_server_test.cc index c1e5d7f4..8cfbb45f 100644 --- a/util/win/exception_handler_server_test.cc +++ b/util/win/exception_handler_server_test.cc @@ -21,6 +21,7 @@ #include "base/basictypes.h" #include "base/strings/stringprintf.h" +#include "base/strings/utf_string_conversions.h" #include "client/crashpad_client.h" #include "gtest/gtest.h" #include "test/win/win_child_process.h" @@ -134,7 +135,7 @@ TEST_F(ExceptionHandlerServerTest, StopWhileConnected) { &server(), &server_thread()); ASSERT_NO_FATAL_FAILURE(delegate().WaitForStart()); CrashpadClient client; - client.SetHandler(pipe_name()); // Connect to server. + client.SetHandlerIPCPipe(base::UTF8ToUTF16(pipe_name())); // Leaving this scope causes the server to be stopped, while the connection // is still open. } @@ -161,9 +162,9 @@ class TestClient final : public WinChildProcess { private: int Run() override { - std::string pipe_name = ReadString(ReadPipeHandle()); + std::wstring pipe_name = base::UTF8ToUTF16(ReadString(ReadPipeHandle())); CrashpadClient client; - if (!client.SetHandler(pipe_name)) { + if (!client.SetHandlerIPCPipe(pipe_name)) { ADD_FAILURE(); return EXIT_FAILURE; }