Patch from Jeremy to have better error reporting, and workaround a Cocoa bug in different locales

R=nealsid
A=jeremy


git-svn-id: http://google-breakpad.googlecode.com/svn/trunk@339 4c0a9323-5329-0410-9bdc-e9ce6186880e

2 files changed, 39 insertions, 16 deletions

diff --git a/src/client/mac/crash_generation/Inspector.mm b/src/client/mac/crash_generation/Inspector.mm
index 8d9c285f..81db9741 100644
--- a/src/client/mac/crash_generation/Inspector.mm
+++ b/src/client/mac/crash_generation/Inspector.mm
@@ -360,7 +360,7 @@ void Inspector::SetCrashTimeParameters() {
 bool Inspector::InspectTask() {
   // keep the task quiet while we're looking at it
   task_suspend(remote_task_);
-  DEBUGLOG(stderr, "Suspsended Remote task\n");
+  DEBUGLOG(stderr, "Suspended Remote task\n");
 
   NSString *minidumpDir;
 
@@ -377,13 +377,19 @@ bool Inspector::InspectTask() {
                                           YES);
 
     NSString *applicationSupportDirectory =
-      [libraryDirectories objectAtIndex:0];
-
-    minidumpDir =
-      [NSString stringWithFormat:@"%@/%s/%s",
-                applicationSupportDirectory,
-                kDefaultLibrarySubdirectory,
-                config_params_.GetValueForKey(BREAKPAD_PRODUCT)];
+        [libraryDirectories objectAtIndex:0];
+    NSString *library_subdirectory = [NSString 
+        stringWithUTF8String:kDefaultLibrarySubdirectory];
+    NSString *breakpad_product = [NSString 
+        stringWithUTF8String:config_params_.GetValueForKey(BREAKPAD_PRODUCT)];
+        
+    NSArray *path_components = [NSArray
+        arrayWithObjects:applicationSupportDirectory,
+                         library_subdirectory,
+                         breakpad_product,
+                         nil];
+
+    minidumpDir = [NSString pathWithComponents:path_components];
   } else {
     minidumpDir = [[NSString stringWithUTF8String:minidumpDirectory]
                     stringByExpandingTildeInPath];
@@ -394,6 +400,24 @@ bool Inspector::InspectTask() {
 
   MinidumpLocation minidumpLocation(minidumpDir);
 
+  // Obscure bug alert:
+  // Don't use [NSString stringWithFormat] to build up the path here since it
+  // assumes system encoding and in RTL locales will prepend an LTR override
+  // character for paths beginning with '/' which fileSystemRepresentation does
+  // not remove. Filed as rdar://6889706 .
+  NSString *path_ns = [NSString
+      stringWithUTF8String:minidumpLocation.GetPath()];
+  NSString *pathid_ns = [NSString
+      stringWithUTF8String:minidumpLocation.GetID()];
+  NSString *minidumpPath = [path_ns stringByAppendingPathComponent:pathid_ns];
+  minidumpPath = [minidumpPath 
+      stringByAppendingPathExtension:@"dmp"];
+  
+  DEBUGLOG(stderr, 
+           "minidump path (%s)\n",
+           [minidumpPath UTF8String]);
+
+
   config_file_.WriteFile( &config_params_,
                           minidumpLocation.GetPath(),
                           minidumpLocation.GetID());
@@ -407,16 +431,14 @@ bool Inspector::InspectTask() {
                                       crashing_thread_);
   }
 
-  NSString *minidumpPath = [NSString stringWithFormat:@"%s/%s.dmp",
-    minidumpLocation.GetPath(), minidumpLocation.GetID()];
-    DEBUGLOG(stderr, 
-           "minidump path (%s)\n",
-           [minidumpPath UTF8String]);
-
 
   bool result = generator.Write([minidumpPath fileSystemRepresentation]);
 
-  DEBUGLOG(stderr, "Wrote minidump - %s\n", result ? "OK" : "FAILED");
+  if (result) {
+    DEBUGLOG(stderr, "Wrote minidump - OK\n");
+  } else {
+    DEBUGLOG(stderr, "Error writing minidump - errno=%s\n",  strerror(errno));
+  }
 
   // let the task continue
   task_resume(remote_task_);
diff --git a/src/client/mac/handler/minidump_generator.cc b/src/client/mac/handler/minidump_generator.cc
index b3091d4d..13db2474 100644
--- a/src/client/mac/handler/minidump_generator.cc
+++ b/src/client/mac/handler/minidump_generator.cc
@@ -172,7 +172,7 @@ bool MinidumpGenerator::Write(const char *path) {
     // exception.
     &MinidumpGenerator::WriteExceptionStream,
   };
-  bool result = true;
+  bool result = false;
 
   // If opening was successful, create the header, directory, and call each
   // writer.  The destructor for the TypedMDRVAs will cause the data to be
@@ -203,6 +203,7 @@ bool MinidumpGenerator::Write(const char *path) {
     header_ptr->stream_directory_rva = dir.position();
 
     MDRawDirectory local_dir;
+    result = true;
     for (int i = 0; (result) && (i < writer_count); ++i) {
       result = (this->*writers[i])(&local_dir);