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.



lldb-commits mailing list

Reply via email to