Linux Breakpad: Keep reading from the crash upload progress until a complete crash id has been rece…

components/breakpad/app/breakpad_linux.cc

Index: components/breakpad/app/breakpad_linux.cc
===================================================================
--- components/breakpad/app/breakpad_linux.cc	(revision 265558)
+++ components/breakpad/app/breakpad_linux.cc	(working copy)
@@ -1020,7 +1020,23 @@
   sys__exit(1);
 }
 
+bool IsValidCrashReportId(const char* buf, size_t buflen, size_t expected_len) {
+  if (buflen != expected_len)
+    return false;
 #if defined(OS_CHROMEOS)
+  return strcmp(buf, "_sys_cr_finished") == 0;
+#else
+  for (size_t i = 0; i < buflen; ++i) {
+    char c = buf[i];

[vapier, 2014/04/23 22:03:21]

i think you want:

 for (size_t i = 0; i < buflen; ++i)
   if (!isxdigit(buf[i]))
     return false;
 return true;

is there not a libbase func to do this too ?

[Lei Zhang, 2014/04/24 00:37:25]

We can't call into libc in the middle of a crash. Note the numerous my_foo
functions used in this file.

+    if ((c >= '0' && c <= '9') || (c >= 'a' || c <= 'f'))
+      continue;
+    return false;
+  }
+  return true;
+#endif
+}
+
+#if defined(OS_CHROMEOS)
 const char* GetCrashingProcessName(const BreakpadInfo& info,
                                    google_breakpad::PageAllocator* allocator) {
   // Symlink to process binary is at /proc/###/exe.
@@ -1409,9 +1425,10 @@
 
       // Helper process.
       if (upload_child > 0) {
+        const size_t kCrashIdLength = 16;
         IGNORE_RET(sys_close(fds[1]));
-        char id_buf[17];  // Crash report IDs are expected to be 16 chars.
-        ssize_t len = -1;
+        char id_buf[kCrashIdLength + 1];
+        size_t bytes_read = 0;
         // Upload should finish in about 10 seconds. Add a few more 500 ms
         // internals to account for process startup time.
         for (size_t wait_count = 0; wait_count < 24; ++wait_count) {
@@ -1424,14 +1441,22 @@
             break;
           } else if (ret > 0) {
             // There is data to read.
-            len = HANDLE_EINTR(sys_read(fds[0], id_buf, sizeof(id_buf) - 1));
-            break;
+            ssize_t len = HANDLE_EINTR(
+                sys_read(fds[0],
+                         &id_buf[bytes_read],
+                         sizeof(id_buf) - bytes_read - 1));
+            if (len < 0)
+              break;
+            bytes_read += len;
+            if (bytes_read == kCrashIdLength)
+              break;
           }
-          // ret == 0 -> timed out, continue waiting.
+          // |ret| == 0 -> timed out, continue waiting.
+          // or |bytes_read| < |kCrashIdLength| still, keep reading.
         }
-        if (len > 0) {
+        id_buf[kCrashIdLength] = 0;  // Always NULL terminate the buffer.
+        if (IsValidCrashReportId(id_buf, bytes_read, kCrashIdLength)) {
           // Write crash dump id to stderr.
-          id_buf[len] = 0;
           static const char msg[] = "\nCrash dump id: ";
           WriteLog(msg, sizeof(msg) - 1);
           WriteLog(id_buf, my_strlen(id_buf));
@@ -1456,7 +1481,11 @@
               IGNORE_RET(sys_close(log_fd));
             }
           }
+        } else {
+          static const char msg[] = "\nFailed to get crash dump id.\n";
+          WriteLog(msg, sizeof(msg) - 1);
         }
+
         if (sys_waitpid(upload_child, NULL, WNOHANG) == 0) {
           // Upload process is still around, kill it.
           sys_kill(upload_child, SIGKILL);