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

Reply via email to