Add a way for the client apps to specify custom information in case of out-of-process

scenarios that the OOP server can use in whatever way it wants to.

Fix a bug in CrashGenerationserver where CreateNamedPipe failure was not checked correctly.

TODO in near future: Add a custom stream to minidump files for the custom information.



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

5 files changed, 80 insertions, 17 deletions

diff --git a/src/client/windows/crash_generation/client_info.cc b/src/client/windows/crash_generation/client_info.cc
index 35dd27b0..48886d2a 100644
--- a/src/client/windows/crash_generation/client_info.cc
+++ b/src/client/windows/crash_generation/client_info.cc
@@ -28,6 +28,7 @@
 // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
 #include "client/windows/crash_generation/client_info.h"
+#include "client/windows/common/ipc_protocol.h"
 
 namespace google_breakpad {
 
@@ -36,12 +37,14 @@ ClientInfo::ClientInfo(CrashGenerationServer* crash_server,
                        MINIDUMP_TYPE dump_type,
                        DWORD* thread_id,
                        EXCEPTION_POINTERS** ex_info,
-                       MDRawAssertionInfo* assert_info)
+                       MDRawAssertionInfo* assert_info,
+                       const CustomClientInfo& custom_client_info)
     : crash_server_(crash_server),
       pid_(pid),
       dump_type_(dump_type),
       ex_info_(ex_info),
       assert_info_(assert_info),
+      custom_client_info_(custom_client_info),
       thread_id_(thread_id),
       process_handle_(NULL),
       dump_requested_handle_(NULL),
@@ -117,8 +120,7 @@ bool ClientInfo::UnregisterWaits() {
   return success;
 }
 
-bool ClientInfo::GetClientExceptionInfo(
-    EXCEPTION_POINTERS** ex_info) const {
+bool ClientInfo::GetClientExceptionInfo(EXCEPTION_POINTERS** ex_info) const {
   SIZE_T bytes_count = 0;
   if (!ReadProcessMemory(process_handle_,
                          ex_info_,
@@ -144,4 +146,32 @@ bool ClientInfo::GetClientThreadId(DWORD* thread_id) const {
   return bytes_count == sizeof(*thread_id);
 }
 
+bool ClientInfo::PopulateCustomInfo() {
+  SIZE_T bytes_count = 0;
+  SIZE_T read_count = sizeof(CustomInfoEntry) * custom_client_info_.count;
+
+  // If the scoped array for custom info already has an array, it will be
+  // the same size as what we need. This is because the number of custom info
+  // entries is always the same. So allocate memory only if scoped array has
+  // a NULL pointer.
+  if (!custom_info_entries_.get()) {
+    custom_info_entries_.reset(new CustomInfoEntry[custom_client_info_.count]);
+  }
+
+  if (!ReadProcessMemory(process_handle_,
+                         custom_client_info_.entries,
+                         custom_info_entries_.get(),
+                         read_count,
+                         &bytes_count)) {
+    return false;
+  }
+
+  return bytes_count == read_count;
+}
+
+int ClientInfo::GetCustomInfo(CustomInfoEntry const** custom_info) const {
+  *custom_info = custom_info_entries_.get();
+  return custom_client_info_.count;
+}
+
 }  // namespace google_breakpad
diff --git a/src/client/windows/crash_generation/client_info.h b/src/client/windows/crash_generation/client_info.h
index 9956c47a..db3f05e9 100644
--- a/src/client/windows/crash_generation/client_info.h
+++ b/src/client/windows/crash_generation/client_info.h
@@ -32,7 +32,9 @@
 
 #include <Windows.h>
 #include <DbgHelp.h>
+#include "client/windows/common/ipc_protocol.h"
 #include "google_breakpad/common/minidump_format.h"
+#include "processor/scoped_ptr.h"
 
 namespace google_breakpad {
 
@@ -49,7 +51,8 @@ class ClientInfo {
              MINIDUMP_TYPE dump_type,
              DWORD* thread_id,
              EXCEPTION_POINTERS** ex_info,
-             MDRawAssertionInfo* assert_info);
+             MDRawAssertionInfo* assert_info,
+             const CustomClientInfo& custom_client_info);
 
   ~ClientInfo();
 
@@ -85,6 +88,10 @@ class ClientInfo {
   bool Initialize();
   bool GetClientExceptionInfo(EXCEPTION_POINTERS** ex_info) const;
   bool GetClientThreadId(DWORD* thread_id) const;
+  // Reads the custom information from the client process address space.
+  bool PopulateCustomInfo();
+  // Returns the client custom information.
+  int GetCustomInfo(CustomInfoEntry const** custom_info) const;
 
  private:
   // Crash generation server.
@@ -113,6 +120,14 @@ class ClientInfo {
   // in the address space of another process.
   MDRawAssertionInfo* assert_info_;
 
+  // Custom information about the client.
+  CustomClientInfo custom_client_info_;
+
+  // Contains the custom client info entries read from the client process
+  // memory. This will be populated only if the method GetClientCustomInfo
+  // is called.
+  scoped_array<CustomInfoEntry> custom_info_entries_;
+
   // Address of a variable in the client process address space that
   // will contain the thread id of the crashing client thread.
   //
diff --git a/src/client/windows/crash_generation/crash_generation_client.cc b/src/client/windows/crash_generation/crash_generation_client.cc
index 4ec932c3..197807ab 100644
--- a/src/client/windows/crash_generation/crash_generation_client.cc
+++ b/src/client/windows/crash_generation/crash_generation_client.cc
@@ -29,6 +29,7 @@
 
 #include "client/windows/crash_generation/crash_generation_client.h"
 #include <cassert>
+#include <utility>
 #include "client/windows/common/ipc_protocol.h"
 
 namespace google_breakpad {
@@ -89,17 +90,23 @@ static bool TransactNamedPipeDebugHelper(HANDLE pipe,
 }
 **/
 
-CrashGenerationClient::CrashGenerationClient(const wchar_t* pipe_name,
-                                             MINIDUMP_TYPE dump_type)
-    : pipe_name_(pipe_name),
-      dump_type_(dump_type),
-      thread_id_(0),
-      server_process_id_(0),
-      crash_event_(NULL),
-      crash_generated_(NULL),
-      server_alive_(NULL),
-      exception_pointers_(NULL) {
+CrashGenerationClient::CrashGenerationClient(
+    const wchar_t* pipe_name,
+    MINIDUMP_TYPE dump_type,
+    const CustomClientInfo* custom_info)
+        : pipe_name_(pipe_name),
+          dump_type_(dump_type),
+          thread_id_(0),
+          server_process_id_(0),
+          crash_event_(NULL),
+          crash_generated_(NULL),
+          server_alive_(NULL),
+          exception_pointers_(NULL),
+          custom_info_() {
   memset(&assert_info_, 0, sizeof(assert_info_));
+  if (custom_info) {
+    custom_info_ = *custom_info;
+  }
 }
 
 CrashGenerationClient::~CrashGenerationClient() {
@@ -184,6 +191,7 @@ bool CrashGenerationClient::RegisterClient(HANDLE pipe) {
                       &thread_id_,
                       &exception_pointers_,
                       &assert_info_,
+                      custom_info_,
                       NULL,
                       NULL,
                       NULL);
diff --git a/src/client/windows/crash_generation/crash_generation_client.h b/src/client/windows/crash_generation/crash_generation_client.h
index fdec1c26..81b0e6ca 100644
--- a/src/client/windows/crash_generation/crash_generation_client.h
+++ b/src/client/windows/crash_generation/crash_generation_client.h
@@ -33,10 +33,14 @@
 #include <windows.h>
 #include <dbghelp.h>
 #include <string>
+#include <utility>
 #include "client/windows/common/ipc_protocol.h"
+#include "processor/scoped_ptr.h"
 
 namespace google_breakpad {
 
+struct CustomClientInfo;
+
 // Abstraction of client-side implementation of out of process
 // crash generation.
 //
@@ -59,7 +63,8 @@ namespace google_breakpad {
 class CrashGenerationClient {
  public:
   CrashGenerationClient(const wchar_t* pipe_name,
-                        MINIDUMP_TYPE dump_type);
+                        MINIDUMP_TYPE dump_type,
+                        const CustomClientInfo* custom_info);
 
   ~CrashGenerationClient();
 
@@ -115,6 +120,9 @@ class CrashGenerationClient {
   // Pipe name to use to talk to server.
   std::wstring pipe_name_;
 
+  // Custom client information
+  CustomClientInfo custom_info_;
+
   // Type of dump to generate.
   MINIDUMP_TYPE dump_type_;
 
diff --git a/src/client/windows/crash_generation/crash_generation_server.cc b/src/client/windows/crash_generation/crash_generation_server.cc
index 9be11ee7..b13db9db 100644
--- a/src/client/windows/crash_generation/crash_generation_server.cc
+++ b/src/client/windows/crash_generation/crash_generation_server.cc
@@ -220,7 +220,7 @@ bool CrashGenerationServer::Start() {
                           kInBufferSize,
                           0,
                           pipe_sec_attrs_);
-  if (!pipe_) {
+  if (pipe_ == INVALID_HANDLE_VALUE) {
     return false;
   }
 
@@ -400,7 +400,8 @@ void CrashGenerationServer::HandleReadDoneState() {
                      msg_.dump_type,
                      msg_.thread_id,
                      msg_.exception_pointers,
-                     msg_.assert_info));
+                     msg_.assert_info,
+                     msg_.custom_client_info));
 
   if (!client_info->Initialize()) {
     server_state_ = IPC_SERVER_STATE_DISCONNECTING;
@@ -726,6 +727,7 @@ void CALLBACK CrashGenerationServer::OnPipeConnected(void* context, BOOLEAN) {
 void CALLBACK CrashGenerationServer::OnDumpRequest(void* context, BOOLEAN) {
   assert(context);
   ClientInfo* client_info = reinterpret_cast<ClientInfo*>(context);
+  client_info->PopulateCustomInfo();
 
   CrashGenerationServer* crash_server = client_info->crash_server();
   assert(crash_server);