From 405bb7aff734100b8d25adfd93d42d54a778b9a4 Mon Sep 17 00:00:00 2001 From: "mark@chromium.org" Date: Wed, 5 Oct 2011 22:32:27 +0000 Subject: [PATCH] Address review comments from r843 (http://breakpad.appspot.com/307001) Review URL: http://breakpad.appspot.com/308001 git-svn-id: http://google-breakpad.googlecode.com/svn/trunk@844 4c0a9323-5329-0410-9bdc-e9ce6186880e --- src/client/mac/Framework/Breakpad.h | 4 ++++ src/client/mac/Framework/OnDemandServer.mm | 13 ++++++------ src/client/mac/crash_generation/Inspector.h | 2 +- src/client/mac/crash_generation/Inspector.mm | 21 ++++++++++++-------- 4 files changed, 25 insertions(+), 15 deletions(-) diff --git a/src/client/mac/Framework/Breakpad.h b/src/client/mac/Framework/Breakpad.h index fa861431..29bb3ec1 100644 --- a/src/client/mac/Framework/Breakpad.h +++ b/src/client/mac/Framework/Breakpad.h @@ -94,6 +94,10 @@ extern "C" { #define BREAKPAD_SERVER_PARAMETER_PREFIX "BreakpadServerParameterPrefix_" #define BREAKPAD_ON_DEMAND "BreakpadOnDemand" +// A service name associated with the original bootstrap parent port, saved in +// OnDemandServer and restored in Inspector. +#define BREAKPAD_BOOTSTRAP_PARENT_PORT "com.Breakpad.BootstrapParent" + // Optional user-defined function to dec to decide if we should handle // this crash or forward it along. // Return true if you want Breakpad to handle it. diff --git a/src/client/mac/Framework/OnDemandServer.mm b/src/client/mac/Framework/OnDemandServer.mm index f736abd4..43a2703b 100644 --- a/src/client/mac/Framework/OnDemandServer.mm +++ b/src/client/mac/Framework/OnDemandServer.mm @@ -28,6 +28,7 @@ // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #import "OnDemandServer.h" +#import "Breakpad.h" #if DEBUG #define PRINT_MACH_RESULT(result_, message_) \ @@ -81,7 +82,7 @@ kern_return_t OnDemandServer::Initialize(const char *server_command, mach_port_t bootstrap_subset_port; kr = bootstrap_subset(bootstrap_port, self_task, &bootstrap_subset_port); - if (kr != KERN_SUCCESS) { + if (kr != BOOTSTRAP_SUCCESS) { PRINT_BOOTSTRAP_RESULT(kr, "bootstrap_subset(): "); return kr; } @@ -94,9 +95,9 @@ kern_return_t OnDemandServer::Initialize(const char *server_command, // recover this port and set it as its own bootstrap port in Inspector.mm // Inspector::ResetBootstrapPort. kr = bootstrap_register(bootstrap_subset_port, - const_cast("BootstrapParentPort"), + const_cast(BREAKPAD_BOOTSTRAP_PARENT_PORT), bootstrap_port); - if (kr != KERN_SUCCESS) { + if (kr != BOOTSTRAP_SUCCESS) { PRINT_BOOTSTRAP_RESULT(kr, "bootstrap_register(): "); return kr; } @@ -106,7 +107,7 @@ kern_return_t OnDemandServer::Initialize(const char *server_command, geteuid(), // server uid true, &server_port_); - if (kr != KERN_SUCCESS) { + if (kr != BOOTSTRAP_SUCCESS) { PRINT_BOOTSTRAP_RESULT(kr, "bootstrap_create_server(): "); return kr; } @@ -118,13 +119,13 @@ kern_return_t OnDemandServer::Initialize(const char *server_command, kr = bootstrap_create_service(server_port_, const_cast(service_name), &service_port_); - if (kr != KERN_SUCCESS) { + if (kr != BOOTSTRAP_SUCCESS) { PRINT_BOOTSTRAP_RESULT(kr, "bootstrap_create_service(): "); // perhaps the service has already been created - try to look it up kr = bootstrap_look_up(bootstrap_port, (char*)service_name, &service_port_); - if (kr != KERN_SUCCESS) { + if (kr != BOOTSTRAP_SUCCESS) { PRINT_BOOTSTRAP_RESULT(kr, "bootstrap_look_up(): "); Unregister(); // clean up server port return kr; diff --git a/src/client/mac/crash_generation/Inspector.h b/src/client/mac/crash_generation/Inspector.h index e6705819..4148eac2 100644 --- a/src/client/mac/crash_generation/Inspector.h +++ b/src/client/mac/crash_generation/Inspector.h @@ -175,7 +175,7 @@ class Inspector { // (ensuring that children like the sender will inherit it), and saves the // subset in bootstrap_subset_port_ for use by ServiceCheckIn and // ServiceCheckOut. - void ResetBootstrapPort(); + kern_return_t ResetBootstrapPort(); kern_return_t ServiceCheckIn(const char *receive_port_name); kern_return_t ServiceCheckOut(const char *receive_port_name); diff --git a/src/client/mac/crash_generation/Inspector.mm b/src/client/mac/crash_generation/Inspector.mm index 77187082..47d3e7b7 100644 --- a/src/client/mac/crash_generation/Inspector.mm +++ b/src/client/mac/crash_generation/Inspector.mm @@ -203,9 +203,12 @@ void ConfigFile::WriteFile(const SimpleStringDictionary *configurationParameters //============================================================================= void Inspector::Inspect(const char *receive_port_name) { - ResetBootstrapPort(); + kern_return_t result = ResetBootstrapPort(); + if (result != KERN_SUCCESS) { + return; + } - kern_return_t result = ServiceCheckIn(receive_port_name); + result = ServiceCheckIn(receive_port_name); if (result == KERN_SUCCESS) { result = ReadMessages(); @@ -243,7 +246,7 @@ void Inspector::Inspect(const char *receive_port_name) { } //============================================================================= -void Inspector::ResetBootstrapPort() { +kern_return_t Inspector::ResetBootstrapPort() { // A reasonable default, in case anything fails. bootstrap_subset_port_ = bootstrap_port; @@ -254,29 +257,31 @@ void Inspector::ResetBootstrapPort() { if (kr != KERN_SUCCESS) { NSLog(@"ResetBootstrapPort: task_get_bootstrap_port failed: %s (%d)", mach_error_string(kr), kr); - return; + return kr; } mach_port_t bootstrap_parent_port; kr = bootstrap_look_up(bootstrap_subset_port_, - "BootstrapParentPort", + const_cast(BREAKPAD_BOOTSTRAP_PARENT_PORT), &bootstrap_parent_port); - if (kr != KERN_SUCCESS) { + if (kr != BOOTSTRAP_SUCCESS) { NSLog(@"ResetBootstrapPort: bootstrap_look_up failed: %s (%d)", bootstrap_strerror(kr), kr); - return; + return kr; } kr = task_set_bootstrap_port(self_task, bootstrap_parent_port); if (kr != KERN_SUCCESS) { NSLog(@"ResetBootstrapPort: task_set_bootstrap_port failed: %s (%d)", mach_error_string(kr), kr); - return; + return kr; } // Some things access the bootstrap port through this global variable // instead of calling task_get_bootstrap_port. bootstrap_port = bootstrap_parent_port; + + return KERN_SUCCESS; } //=============================================================================