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
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
