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 {
+public:
----------------
Arguably, this should be renamed to ProcessDebuggerData, but that's not a
sticking point right now.
Repository:
rLLDB LLDB
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D63166/new/
https://reviews.llvm.org/D63166
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
