From b4675a4e12d7120b1b81c41b63b9da55265810a1 Mon Sep 17 00:00:00 2001 From: Enrico Granata Date: Tue, 25 Jun 2013 23:43:28 +0000 Subject: [PATCH] The semi-unofficial way of returning a status from a Python command was to return a string (e.g. return "no such variable was found") that LLDB would pick as a clue of an error having happened This checkin changes that: - SBCommandReturnObject now exports a SetError() call, which can take an SBError or a plain C-string - script commands now drop any return value and expect the SBCommandReturnObject ("return object") to be filled in appropriately - if you do nothing, a success will be assumed If your commands were relying on returning a value and having LLDB pick that up as an error, please change your commands to SetError() through the return object or expect changes in behavior llvm-svn: 184893 --- lldb/include/lldb/API/SBCommandReturnObject.h | 7 ++++++ lldb/include/lldb/API/SBError.h | 1 + .../lldb/Interpreter/CommandReturnObject.h | 5 +++- .../lldb/Interpreter/ScriptInterpreter.h | 1 - lldb/lldb.xcodeproj/project.pbxproj | 2 +- .../Python/interface/SBCommandReturnObject.i | 7 ++++++ lldb/scripts/Python/python-wrapper.swig | 25 +++---------------- lldb/source/API/SBCommandReturnObject.cpp | 20 +++++++++++++++ .../source/Commands/CommandObjectCommands.cpp | 4 ++- .../Interpreter/CommandReturnObject.cpp | 14 +++++++++-- .../Interpreter/ScriptInterpreterPython.cpp | 6 ++--- .../functionalities/command_script/welcome.py | 7 ++---- 12 files changed, 62 insertions(+), 37 deletions(-) diff --git a/lldb/include/lldb/API/SBCommandReturnObject.h b/lldb/include/lldb/API/SBCommandReturnObject.h index cf04a3eab3fb..f2d274802330 100644 --- a/lldb/include/lldb/API/SBCommandReturnObject.h +++ b/lldb/include/lldb/API/SBCommandReturnObject.h @@ -98,6 +98,13 @@ public: const char * GetError (bool only_if_no_immediate); + void + SetError (lldb::SBError &error, + const char *fallback_error_cstr = NULL); + + void + SetError (const char* error_cstr); + protected: friend class SBCommandInterpreter; friend class SBOptions; diff --git a/lldb/include/lldb/API/SBError.h b/lldb/include/lldb/API/SBError.h index fbfb34d24bf0..a6d3dacb4549 100644 --- a/lldb/include/lldb/API/SBError.h +++ b/lldb/include/lldb/API/SBError.h @@ -66,6 +66,7 @@ public: protected: + friend class SBCommandReturnObject; friend class SBData; friend class SBDebugger; friend class SBCommunication; diff --git a/lldb/include/lldb/Interpreter/CommandReturnObject.h b/lldb/include/lldb/Interpreter/CommandReturnObject.h index aa92e14c1734..0c831408a1b0 100644 --- a/lldb/include/lldb/Interpreter/CommandReturnObject.h +++ b/lldb/include/lldb/Interpreter/CommandReturnObject.h @@ -143,8 +143,11 @@ public: void SetError (const Error &error, - const char *fallback_error_cstr); + const char *fallback_error_cstr = NULL); + void + SetError (const char *error_cstr); + lldb::ReturnStatus GetStatus(); diff --git a/lldb/include/lldb/Interpreter/ScriptInterpreter.h b/lldb/include/lldb/Interpreter/ScriptInterpreter.h index 2566c3ff78fc..9a66c775d47a 100644 --- a/lldb/include/lldb/Interpreter/ScriptInterpreter.h +++ b/lldb/include/lldb/Interpreter/ScriptInterpreter.h @@ -120,7 +120,6 @@ public: const char *session_dictionary_name, lldb::DebuggerSP& debugger, const char* args, - std::string& err_msg, lldb_private::CommandReturnObject& cmd_retobj); typedef bool (*SWIGPythonCallModuleInit) (const char *python_module_name, diff --git a/lldb/lldb.xcodeproj/project.pbxproj b/lldb/lldb.xcodeproj/project.pbxproj index 187d24c2a270..1e4544406a30 100644 --- a/lldb/lldb.xcodeproj/project.pbxproj +++ b/lldb/lldb.xcodeproj/project.pbxproj @@ -1656,7 +1656,7 @@ 9A42976211861AA600FE05CD /* CommandObjectBreakpointCommand.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = CommandObjectBreakpointCommand.cpp; path = source/Commands/CommandObjectBreakpointCommand.cpp; sourceTree = ""; }; 9A4633DA11F65D8600955CE1 /* UserSettingsController.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = UserSettingsController.h; path = include/lldb/Core/UserSettingsController.h; sourceTree = ""; }; 9A4633DC11F65D9A00955CE1 /* UserSettingsController.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = UserSettingsController.cpp; path = source/Core/UserSettingsController.cpp; sourceTree = ""; }; - 9A48A3A7124AAA5A00922451 /* python-extensions.swig */ = {isa = PBXFileReference; explicitFileType = sourcecode.cpp.cpp; fileEncoding = 4; path = "python-extensions.swig"; sourceTree = ""; }; + 9A48A3A7124AAA5A00922451 /* python-extensions.swig */ = {isa = PBXFileReference; explicitFileType = text.script.python; fileEncoding = 4; path = "python-extensions.swig"; sourceTree = ""; }; 9A4F350F1368A51A00823F52 /* StreamAsynchronousIO.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = StreamAsynchronousIO.cpp; path = source/Core/StreamAsynchronousIO.cpp; sourceTree = ""; }; 9A4F35111368A54100823F52 /* StreamAsynchronousIO.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = StreamAsynchronousIO.h; path = include/lldb/Core/StreamAsynchronousIO.h; sourceTree = ""; }; 9A633FE7112DCE3C001A7E43 /* SBFrame.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = SBFrame.cpp; path = source/API/SBFrame.cpp; sourceTree = ""; }; diff --git a/lldb/scripts/Python/interface/SBCommandReturnObject.i b/lldb/scripts/Python/interface/SBCommandReturnObject.i index 7cec06943bf8..4026413ffee4 100644 --- a/lldb/scripts/Python/interface/SBCommandReturnObject.i +++ b/lldb/scripts/Python/interface/SBCommandReturnObject.i @@ -58,7 +58,14 @@ public: void SetStatus (lldb::ReturnStatus status); + + void + SetError (lldb::SBError &error, + const char *fallback_error_cstr = NULL); + void + SetError (const char *error_cstr); + lldb::ReturnStatus GetStatus(); diff --git a/lldb/scripts/Python/python-wrapper.swig b/lldb/scripts/Python/python-wrapper.swig index 552b8f637e76..590c4506da9a 100644 --- a/lldb/scripts/Python/python-wrapper.swig +++ b/lldb/scripts/Python/python-wrapper.swig @@ -318,7 +318,7 @@ LLDBSwigPythonCallTypeScript if (!python_function_name || !session_dictionary) return false; - PyObject *session_dict = (PyObject*)session_dictionary, *pfunc_impl = NULL, *pargs = NULL, *pvalue = NULL; + PyObject *session_dict = (PyObject*)session_dictionary, *pfunc_impl = NULL, *pvalue = NULL; if (pyfunct_wrapper && *pyfunct_wrapper && PyFunction_Check (*pyfunct_wrapper)) { @@ -475,10 +475,6 @@ LLDBSwigPython_GetChildAtIndex uint32_t idx ) { - - static char callee_name[] = "get_child_at_index"; - static char param_format[] = "i"; - PyErr_Cleaner py_err_cleaner(true); PyCallable pfunc = PyCallable::FindWithMemberFunction(implementor,"get_child_at_index"); @@ -628,7 +624,6 @@ LLDBSwigPythonCallCommand const char *session_dictionary_name, lldb::DebuggerSP& debugger, const char* args, - std::string& err_msg, lldb_private::CommandReturnObject& cmd_retobj ) { @@ -653,23 +648,9 @@ LLDBSwigPythonCallCommand pvalue = pfunc(debugger_sb, args, &cmd_retobj_sb, session_dict = FindSessionDictionary(session_dictionary_name)); Py_XINCREF (session_dict); - - if (pvalue != NULL) - { - if (pvalue == Py_None) // no error - { - err_msg.clear(); - retval = true; - } - else - { - // return value is an error string - PyObjectToString(pvalue,err_msg); - retval = false; - } - } - Py_XDECREF (pvalue); + + retval = true; } return retval; diff --git a/lldb/source/API/SBCommandReturnObject.cpp b/lldb/source/API/SBCommandReturnObject.cpp index b0533e132db3..e94c01748197 100644 --- a/lldb/source/API/SBCommandReturnObject.cpp +++ b/lldb/source/API/SBCommandReturnObject.cpp @@ -8,8 +8,10 @@ //===----------------------------------------------------------------------===// #include "lldb/API/SBCommandReturnObject.h" +#include "lldb/API/SBError.h" #include "lldb/API/SBStream.h" +#include "lldb/Core/Error.h" #include "lldb/Core/Log.h" #include "lldb/Interpreter/CommandReturnObject.h" @@ -329,3 +331,21 @@ SBCommandReturnObject::Printf(const char* format, ...) return 0; } +void +SBCommandReturnObject::SetError (lldb::SBError &error, const char *fallback_error_cstr) +{ + if (m_opaque_ap.get()) + { + if (error.IsValid()) + m_opaque_ap->SetError(error.ref(), fallback_error_cstr); + else if (fallback_error_cstr) + m_opaque_ap->SetError(Error(), fallback_error_cstr); + } +} + +void +SBCommandReturnObject::SetError (const char *error_cstr) +{ + if (m_opaque_ap.get() && error_cstr) + m_opaque_ap->SetError(error_cstr); +} diff --git a/lldb/source/Commands/CommandObjectCommands.cpp b/lldb/source/Commands/CommandObjectCommands.cpp index 223083677ec6..bbea08518344 100644 --- a/lldb/source/Commands/CommandObjectCommands.cpp +++ b/lldb/source/Commands/CommandObjectCommands.cpp @@ -1330,7 +1330,9 @@ protected: // Don't change the status if the command already set it... if (result.GetStatus() == eReturnStatusInvalid) { - if (result.GetOutputData() == NULL || result.GetOutputData()[0] == '\0') + if (result.GetErrorData() && result.GetErrorData()[0]) + result.SetStatus(eReturnStatusFailed); + else if (result.GetOutputData() == NULL || result.GetOutputData()[0] == '\0') result.SetStatus(eReturnStatusSuccessFinishNoResult); else result.SetStatus(eReturnStatusSuccessFinishResult); diff --git a/lldb/source/Interpreter/CommandReturnObject.cpp b/lldb/source/Interpreter/CommandReturnObject.cpp index 2b93a546f8a9..9c63753a23ff 100644 --- a/lldb/source/Interpreter/CommandReturnObject.cpp +++ b/lldb/source/Interpreter/CommandReturnObject.cpp @@ -143,9 +143,19 @@ CommandReturnObject::SetError (const Error &error, const char *fallback_error_cs const char *error_cstr = error.AsCString(); if (error_cstr == NULL) error_cstr = fallback_error_cstr; - AppendError (error_cstr); - SetStatus (eReturnStatusFailed); + SetError(error_cstr); } + +void +CommandReturnObject::SetError (const char *error_cstr) +{ + if (error_cstr) + { + AppendError (error_cstr); + SetStatus (eReturnStatusFailed); + } +} + // Similar to AppendError, but do not prepend 'Error: ' to message, and // don't append "\n" to the end of it. diff --git a/lldb/source/Interpreter/ScriptInterpreterPython.cpp b/lldb/source/Interpreter/ScriptInterpreterPython.cpp index e42ca6807ee4..4e41d1508425 100644 --- a/lldb/source/Interpreter/ScriptInterpreterPython.cpp +++ b/lldb/source/Interpreter/ScriptInterpreterPython.cpp @@ -115,7 +115,6 @@ LLDBSwigPythonCallCommand (const char *python_function_name, const char *session_dictionary_name, lldb::DebuggerSP& debugger, const char* args, - std::string& err_msg, lldb_private::CommandReturnObject& cmd_retobj); extern "C" bool @@ -2970,7 +2969,7 @@ ScriptInterpreterPython::RunScriptBasedCommand(const char* impl_function, return false; } - bool ret_val; + bool ret_val = false; std::string err_msg; @@ -2995,12 +2994,11 @@ ScriptInterpreterPython::RunScriptBasedCommand(const char* impl_function, m_dictionary_name.c_str(), debugger_sp, args, - err_msg, cmd_retobj); } if (!ret_val) - error.SetErrorString(err_msg.c_str()); + error.SetErrorString("unable to execute script function"); else error.Clear(); diff --git a/lldb/test/functionalities/command_script/welcome.py b/lldb/test/functionalities/command_script/welcome.py index b7e8072aeeb4..b6340990e789 100644 --- a/lldb/test/functionalities/command_script/welcome.py +++ b/lldb/test/functionalities/command_script/welcome.py @@ -13,9 +13,7 @@ def target_name_impl(debugger, args, result, dict): file = target.GetExecutable() print >>result, ('Current target ' + file.GetFilename()) if args == 'fail': - return 'a test for error in command' - else: - return None + result.SetError('a test for error in command') def print_wait_impl(debugger, args, result, dict): result.SetImmediateOutputFile(sys.stdout) @@ -25,11 +23,10 @@ def print_wait_impl(debugger, args, result, dict): print >>result, ('Still doing long task..') time.sleep(1) print >>result, ('Done; if you saw the delays I am doing OK') - return None def check_for_synchro(debugger, args, result, dict): if debugger.GetAsync() == True: print >>result, ('I am running async') if debugger.GetAsync() == False: print >>result, ('I am running sync') - return None +