[Lldb-commits] [PATCH] D63166: Move common functionality from processwindows into processdebugger

Adrian McCarthy via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Fri Jun 14 12:03:19 PDT 2019

amccarth added a comment.

I'm OK with moving common stuff out for now, and I like the separation of ProcessWindows and ProcessDebugger.

I agree that we don't want to live too long in a state with a regular plugin and a remote plugin, I still think there's advantage to having common Windows stuff grouped together (though, perhaps, not exactly this grouping in the long run).  I'm trying to think through the implications on cross-platform post-mortem debugging, e.g., debugging a Windows minidump on a Linux host without spinning up a remote on an actual Windows machine.

This LGTM as an incremental step if you address some of the naming slips and others' feedback.

Comment at: source/Plugins/Process/Windows/Common/ProcessDebugger.cpp:125
+    StreamString stream;
+    stream.Printf("ProcessWindows unable to launch '%s'.  ProcessWindows can "
+                  "only be used for debug launches.",
s/ProcessWindows/ProcessDebugger/  (2x)

Comment at: source/Plugins/Process/Windows/Common/ProcessDebugger.cpp:215
+    // DebuggerThread. StopDebugging() will trigger a call back into
+    // ProcessWindows which will acquire the lock again, so we need to not
+    // deadlock.
s/ProcessWindows/ProcessDebugger/ ?

Comment at: source/Plugins/Process/Windows/Common/ProcessDebugger.h:1
+//===-- NativeProcessWindows.h ----------------------------------*- C++ -*-===//
Please fix file title.

Comment at: source/Plugins/Process/Windows/Common/ProcessDebugger.h:30
+class ProcessWindowsData {
Arguably, this should be renamed to ProcessDebuggerData, but that's not a sticking point right now.




More information about the lldb-commits mailing list