[Lldb-commits] [PATCH] Windows command-line driver - PlatformWindows patch

[João Matos]

  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