Overall it looks good, there's just the ResolveExecutable method which I
think should be split up a bit, but I had already suggested that in the first
review.
There are also some formatting inconsistency, I'd suggest running
clang-format but I'm not sure if the LLDB style is supported yet. About
committing, since you probably will have a lot more patches, it'd be more
convenient for everyone if you could ask for commit access to get these in once
they're reviewed.
================
Comment at: source/Plugins/Platform/Windows/PlatformWindows.cpp:135
@@ +134,3 @@
+ {
+ if ( --g_initialize_count == 0 )
+ {
----------------
Extra spaces around parens.
================
Comment at: source/Plugins/Platform/Windows/PlatformWindows.cpp:118
@@ +117,3 @@
+ WSADATA dummy;
+ WSAStartup( MAKEWORD( 2, 2 ), &dummy);
+ // Force a host flag to true for the default platform object.
----------------
Inconsistent spaces in calls.
================
Comment at: source/lldb.cpp:191
@@ -183,2 +190,3 @@
PluginManager::Terminate();
-
+#if defined( _WIN32 )
+ PlatformWindows::Terminate();
----------------
Extra spaces.
================
Comment at: source/Plugins/Platform/Windows/PlatformWindows.cpp:292
@@ +291,3 @@
+ default:
+ assert(!"Unhandled architecture in
PlatformWindows::GetSoftwareBreakpointTrapOpcode()");
+ break;
----------------
In LLVM it's usually preferred to use llvm_unreachable for these cases.
================
Comment at: source/lldb.cpp:79
@@ -78,1 +78,3 @@
+#if defined (_MSC_VER)
+#include "Plugins/Platform/Windows/PlatformWindows.h"
----------------
Again, should this not be _WIN32 to match the initialization ifdef below?
================
Comment at: source/Plugins/Platform/Windows/PlatformWindows.cpp:498
@@ +497,3 @@
+ debugger.GetTargetList().SetSelectedTarget(target);
+ // The freebsd always currently uses the GDB remote debugger
plug-in
+ // so even when debugging locally we are debugging remotely!
----------------
Are the freebsd references here just copy-paste leftovers? Should the comments
be updated for the Windows platform?
http://llvm-reviews.chandlerc.com/D1772
_______________________________________________
lldb-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
