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

[João Matos]

  First round of comments.


================
Comment at: CMakeLists.txt:272
@@ +271,2 @@
+add_subdirectory(test)
+add_subdirectory(tools)
----------------
Can you fix these to use same newline encodings as the original so we can meaningful diffs?

If you really want to change the newlines, then do it in a separate commit.



================
Comment at: source/Plugins/CMakeLists.txt:13
@@ +12,2 @@
+add_subdirectory(SymbolVendor)
+add_subdirectory(UnwindAssembly)
----------------
Same as above.

================
Comment at: source/Plugins/Platform/Windows/PlatformWindows.cpp:121
@@ +120,3 @@
+        WSADATA dummy;
+        WSAStartup(0x202, &dummy);
+        // Force a host flag to true for the default platform object.
----------------
Use the MAKEWORD(2, 2) macro to make this a bit more clear?

================
Comment at: source/Plugins/Platform/Windows/PlatformWindows.cpp:100
@@ +99,3 @@
+{
+    if (is_host)
+        return "Local Windows user platform plug-in.";
----------------
You could use the conditional operator here

================
Comment at: source/Plugins/Platform/Windows/PlatformWindows.cpp:169
@@ +168,3 @@
+{
+    Error error;
+    // Nothing special to do here, just use the actual file and architecture
----------------
This method is huge, please split it up in some helper functions.

================
Comment at: source/lldb.cpp:80
@@ +79,3 @@
+#if defined (_MSC_VER)
+#include "Plugins/Platform/Windows/PlatformWindows.h"
+#endif
----------------
Any reason we test for _MSC_VER here and not _WIN32?

================
Comment at: source/lldb.cpp:123
@@ -118,1 +122,3 @@
         ItaniumABILanguageRuntime::Initialize();
+#if defined( _WIN32 )
+        PlatformWindows::Initialize();
----------------
Extra spaces between ( )

================
Comment at: source/Plugins/Platform/Windows/PlatformWindows.cpp:302
@@ +301,3 @@
+
+	switch (arch.GetCore())
+	{
----------------
Indenting seems off here.

================
Comment at: source/Plugins/Platform/Windows/PlatformWindows.cpp:328
@@ +327,3 @@
+{
+    if (m_remote_platform_sp)
+        return m_remote_platform_sp->GetOSVersion (m_major_os_version,
----------------
Invert the condition and early return?


http://llvm-reviews.chandlerc.com/D1772