[Lldb-commits] [PATCH] D12126: Read exception records from Windows mini dump

[Adrian McCarthy via lldb-commits]

amccarth marked 2 inline comments as done.

================
Comment at: source/Plugins/Process/win-minidump/ProcessWinMiniDump.cpp:173
@@ -174,1 +172,3 @@
 {
+    if (m_data_up && m_data_up->m_exception_sp)
+    {
----------------
zturner wrote:
> Can you invert this conditional and early-out to keep the indentation level reduced?
Done, but >grumble<.  It seems likely this code will evolve into a cascaded if-else-if (as per the ProcessWindows::RefreshStateAfterStop), in which case the early out would have to be reverted.

Checking for prerequisites before doing something is natural, flexible, and avoids code duplication.  Early outs are like Yoda comparisons--harder to read for dubious advantage.

================
Comment at: source/Plugins/Process/win-minidump/ProcessWinMiniDump.cpp:320
@@ -312,6 +319,3 @@
+    if (system_info_ptr)
     {
         switch (system_info_ptr->ProcessorArchitecture)
----------------
zturner wrote:
> Same here
That would violate the DRY principle, since I'd have to repeat the construction of an invalid or unknown ArchSpec.


http://reviews.llvm.org/D12126