[Lldb-commits] [PATCH] D24711: [lldb-mi] Fix implementation for a few mi commands

[Hafiz Abid Qadeer via lldb-commits]

abidh added a comment.

Changes looks mostly Ok to me apart from some comments. Please address them and add testcases as mentioned by ilia. Also try to do one review for one fix. This review is for 3 fixes. When the changes are approved, please commit them in 3 separate commits (one per fix).


================
Comment at: tools/lldb-mi/MICmdCmdGdbSet.cpp:421
@@ +420,3 @@
+  lldb::SBDebugger &rDbgr = m_rLLDBDebugSessionInfo.GetDebugger();
+  lldb::SBDebugger::SetInternalVariable("target.x86-disassembly-flavor", rStrValDisasmFlavor.c_str(), rDbgr.GetInstanceName());
+
----------------
You are not checking if SetInternalVariable returned an error which can happen in this case if the flavor name was not one of "intel", "att" or "default".

================
Comment at: tools/lldb-mi/MICmdCmdMiscellanous.cpp:515
@@ -507,2 +514,3 @@
+      return MIstatus::failure;
   }
 
----------------
It is not really an OutofBand record but rather an output of the command. Why not simple add prepend an ~

================
Comment at: tools/lldb-mi/MICmdCmdTarget.cpp:126
@@ -125,2 +125,3 @@
   lldb::SBStream errMsg;
+  error.GetDescription(errMsg);
   if (!process.IsValid()) {
----------------
This does not seem related to any bug fix.


Repository:
  rL LLVM

https://reviews.llvm.org/D24711