polyakov.alex added inline comments.
================ Comment at: tools/lldb-mi/MICmdCmdExec.cpp:248 } - } else { - // ToDo: Re-evaluate if this is required when application near finished as - // this is parsing LLDB error message - // which seems a hack and is code brittle - const char *pLldbErr = m_lldbResult.GetError(); - const CMIUtilString strLldbMsg(CMIUtilString(pLldbErr).StripCREndOfLine()); - if (strLldbMsg == "error: Process must be launched.") { - CMIDriver::Instance().SetExitApplicationFlag(true); - } - } + } else m_lldbResult.SetError(error.GetCString()); ---------------- clayborg wrote: > clayborg wrote: > > clayborg wrote: > > > polyakov.alex wrote: > > > > clayborg wrote: > > > > > An error can claim to fail but not have an error string set. It might > > > > > be nice to have a helper function that checks to make sure there is > > > > > an error string on error cases and if there is no error string when > > > > > error.Success() is false or error.Fail() is true, then set the error > > > > > string to "unknown error". > > > > This functionality might be useful in all lldb-mi commands. So do you > > > > know where to place this function? Maybe inside SBError class? > > > I would put it somewhere in lldb-mi in a static function that is > > > something like: > > > ``` > > > static void SetErrorString(lldb::SBError &error, T &lldbResult) { > > > const char *error_cstr = error.GetCString(); > > > if (error_cstr) > > > lldbResult.SetError(error.GetCString()); > > > else > > > lldbResult.SetError("unknown error"); > > > } > > > ``` > > > Where the T is the type of m_lldbResult. > > Actually, are we using m_lldbResult now? I didn't realize its type was > > lldb::SBCommandReturnObject. That was only needed if we were calling: > > ``` > > rSessionInfo.GetDebugger().GetCommandInterpreter().HandleCommand(pCmd, > > m_lldbResult); > > ``` > > > > So we can get rid of all "lldb::SBCommandReturnObject m_lldbResult" member > > variables in any lldb-mi functions where we switch to using the API. > Since we can get rid of m_lldbResult, this should be: > > ``` > } else return MIstatus::failure; > ``` It should be: ``` else { SetError(CMIUtilString::Format(..., error.GetCString())); return MIstatus::failure; } ``` So, we anyway need a C-style error string. https://reviews.llvm.org/D47415 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits