Get rid of the C-string parameter in DoExecute

Summary:
This patch gets rid of the C-string parameter in the RawCommandObject::DoExecute function,
making the code simpler and less memory unsafe.

There seems to be a assumption in some command objects that this parameter could be a nullptr,
but from what I can see the rest of the API doesn't actually allow this (and other command
objects and related code pieces dereference this parameter without any checks).

Especially CommandObjectRegexCommand has error handling code for a nullptr that is now gone.

Reviewers: davide, jingham, teemperor

Reviewed By: teemperor

Subscribers: jingham, lldb-commits

Differential Revision: https://reviews.llvm.org/D49207

llvm-svn: 336955

1 files changed, 121 insertions, 118 deletions

diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
index 2a196ab1cc8..e115057db8c 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -751,12 +751,12 @@ static void ReadThreadBytesReceived(void *baton, const void *src,
 }
 
 bool ScriptInterpreterPython::ExecuteOneLine(
-    const char *command, CommandReturnObject *result,
+    llvm::StringRef command, CommandReturnObject *result,
     const ExecuteScriptOptions &options) {
   if (!m_valid_session)
     return false;
 
-  if (command && command[0]) {
+  if (!command.empty()) {
     // We want to call run_one_line, passing in the dictionary and the command
     // string.  We cannot do this through PyRun_SimpleString here because the
     // command string may contain escaped characters, and putting it inside
@@ -894,9 +894,11 @@ bool ScriptInterpreterPython::ExecuteOneLine(
       return true;
 
     // The one-liner failed.  Append the error message.
-    if (result)
+    if (result) {
+      std::string command_str = command.str();
       result->AppendErrorWithFormat(
-          "python failed attempting to evaluate '%s'\n", command);
+          "python failed attempting to evaluate '%s'\n", command_str.c_str());
+    }
     return false;
   }
 
@@ -1021,7 +1023,7 @@ bool ScriptInterpreterPython::Interrupt() {
   return false;
 }
 bool ScriptInterpreterPython::ExecuteOneLineWithReturn(
-    const char *in_string, ScriptInterpreter::ScriptReturnType return_type,
+    llvm::StringRef in_string, ScriptInterpreter::ScriptReturnType return_type,
     void *ret_value, const ExecuteScriptOptions &options) {
 
   Locker locker(this, ScriptInterpreterPython::Locker::AcquireLock |
@@ -1056,116 +1058,114 @@ bool ScriptInterpreterPython::ExecuteOneLineWithReturn(
   if (py_error.IsValid())
     PyErr_Clear();
 
-  if (in_string != nullptr) {
-    { // scope for PythonInputReaderManager
-      // PythonInputReaderManager py_input(options.GetEnableIO() ? this : NULL);
-      py_return.Reset(
-          PyRefType::Owned,
-          PyRun_String(in_string, Py_eval_input, globals.get(), locals.get()));
-      if (!py_return.IsValid()) {
-        py_error.Reset(PyRefType::Borrowed, PyErr_Occurred());
-        if (py_error.IsValid())
-          PyErr_Clear();
-
-        py_return.Reset(PyRefType::Owned,
-                        PyRun_String(in_string, Py_single_input, globals.get(),
-                                     locals.get()));
-      }
+  std::string as_string = in_string.str();
+  { // scope for PythonInputReaderManager
+    // PythonInputReaderManager py_input(options.GetEnableIO() ? this : NULL);
+    py_return.Reset(PyRefType::Owned,
+                    PyRun_String(as_string.c_str(), Py_eval_input,
+                                 globals.get(), locals.get()));
+    if (!py_return.IsValid()) {
+      py_error.Reset(PyRefType::Borrowed, PyErr_Occurred());
+      if (py_error.IsValid())
+        PyErr_Clear();
+
+      py_return.Reset(PyRefType::Owned,
+                      PyRun_String(as_string.c_str(), Py_single_input,
+                                   globals.get(), locals.get()));
     }
+  }
 
-    if (py_return.IsValid()) {
-      switch (return_type) {
-      case eScriptReturnTypeCharPtr: // "char *"
-      {
-        const char format[3] = "s#";
-        success = PyArg_Parse(py_return.get(), format, (char **)ret_value);
-        break;
-      }
-      case eScriptReturnTypeCharStrOrNone: // char* or NULL if py_return ==
-                                           // Py_None
-        {
-          const char format[3] = "z";
-          success = PyArg_Parse(py_return.get(), format, (char **)ret_value);
-          break;
-        }
-      case eScriptReturnTypeBool: {
-        const char format[2] = "b";
-        success = PyArg_Parse(py_return.get(), format, (bool *)ret_value);
-        break;
-      }
-      case eScriptReturnTypeShortInt: {
-        const char format[2] = "h";
-        success = PyArg_Parse(py_return.get(), format, (short *)ret_value);
-        break;
-      }
-      case eScriptReturnTypeShortIntUnsigned: {
-        const char format[2] = "H";
-        success =
-            PyArg_Parse(py_return.get(), format, (unsigned short *)ret_value);
-        break;
-      }
-      case eScriptReturnTypeInt: {
-        const char format[2] = "i";
-        success = PyArg_Parse(py_return.get(), format, (int *)ret_value);
-        break;
-      }
-      case eScriptReturnTypeIntUnsigned: {
-        const char format[2] = "I";
-        success =
-            PyArg_Parse(py_return.get(), format, (unsigned int *)ret_value);
-        break;
-      }
-      case eScriptReturnTypeLongInt: {
-        const char format[2] = "l";
-        success = PyArg_Parse(py_return.get(), format, (long *)ret_value);
-        break;
-      }
-      case eScriptReturnTypeLongIntUnsigned: {
-        const char format[2] = "k";
-        success =
-            PyArg_Parse(py_return.get(), format, (unsigned long *)ret_value);
-        break;
-      }
-      case eScriptReturnTypeLongLong: {
-        const char format[2] = "L";
-        success = PyArg_Parse(py_return.get(), format, (long long *)ret_value);
-        break;
-      }
-      case eScriptReturnTypeLongLongUnsigned: {
-        const char format[2] = "K";
-        success = PyArg_Parse(py_return.get(), format,
-                              (unsigned long long *)ret_value);
-        break;
-      }
-      case eScriptReturnTypeFloat: {
-        const char format[2] = "f";
-        success = PyArg_Parse(py_return.get(), format, (float *)ret_value);
-        break;
-      }
-      case eScriptReturnTypeDouble: {
-        const char format[2] = "d";
-        success = PyArg_Parse(py_return.get(), format, (double *)ret_value);
-        break;
-      }
-      case eScriptReturnTypeChar: {
-        const char format[2] = "c";
-        success = PyArg_Parse(py_return.get(), format, (char *)ret_value);
-        break;
-      }
-      case eScriptReturnTypeOpaqueObject: {
-        success = true;
-        PyObject *saved_value = py_return.get();
-        Py_XINCREF(saved_value);
-        *((PyObject **)ret_value) = saved_value;
-        break;
-      }
-      }
-
-      if (success)
-        ret_success = true;
-      else
-        ret_success = false;
+  if (py_return.IsValid()) {
+    switch (return_type) {
+    case eScriptReturnTypeCharPtr: // "char *"
+    {
+      const char format[3] = "s#";
+      success = PyArg_Parse(py_return.get(), format, (char **)ret_value);
+      break;
+    }
+    case eScriptReturnTypeCharStrOrNone: // char* or NULL if py_return ==
+                                         // Py_None
+    {
+      const char format[3] = "z";
+      success = PyArg_Parse(py_return.get(), format, (char **)ret_value);
+      break;
+    }
+    case eScriptReturnTypeBool: {
+      const char format[2] = "b";
+      success = PyArg_Parse(py_return.get(), format, (bool *)ret_value);
+      break;
+    }
+    case eScriptReturnTypeShortInt: {
+      const char format[2] = "h";
+      success = PyArg_Parse(py_return.get(), format, (short *)ret_value);
+      break;
+    }
+    case eScriptReturnTypeShortIntUnsigned: {
+      const char format[2] = "H";
+      success =
+          PyArg_Parse(py_return.get(), format, (unsigned short *)ret_value);
+      break;
     }
+    case eScriptReturnTypeInt: {
+      const char format[2] = "i";
+      success = PyArg_Parse(py_return.get(), format, (int *)ret_value);
+      break;
+    }
+    case eScriptReturnTypeIntUnsigned: {
+      const char format[2] = "I";
+      success = PyArg_Parse(py_return.get(), format, (unsigned int *)ret_value);
+      break;
+    }
+    case eScriptReturnTypeLongInt: {
+      const char format[2] = "l";
+      success = PyArg_Parse(py_return.get(), format, (long *)ret_value);
+      break;
+    }
+    case eScriptReturnTypeLongIntUnsigned: {
+      const char format[2] = "k";
+      success =
+          PyArg_Parse(py_return.get(), format, (unsigned long *)ret_value);
+      break;
+    }
+    case eScriptReturnTypeLongLong: {
+      const char format[2] = "L";
+      success = PyArg_Parse(py_return.get(), format, (long long *)ret_value);
+      break;
+    }
+    case eScriptReturnTypeLongLongUnsigned: {
+      const char format[2] = "K";
+      success =
+          PyArg_Parse(py_return.get(), format, (unsigned long long *)ret_value);
+      break;
+    }
+    case eScriptReturnTypeFloat: {
+      const char format[2] = "f";
+      success = PyArg_Parse(py_return.get(), format, (float *)ret_value);
+      break;
+    }
+    case eScriptReturnTypeDouble: {
+      const char format[2] = "d";
+      success = PyArg_Parse(py_return.get(), format, (double *)ret_value);
+      break;
+    }
+    case eScriptReturnTypeChar: {
+      const char format[2] = "c";
+      success = PyArg_Parse(py_return.get(), format, (char *)ret_value);
+      break;
+    }
+    case eScriptReturnTypeOpaqueObject: {
+      success = true;
+      PyObject *saved_value = py_return.get();
+      Py_XINCREF(saved_value);
+      *((PyObject **)ret_value) = saved_value;
+      break;
+    }
+    }
+
+    if (success)
+      ret_success = true;
+    else
+      ret_success = false;
   }
 
   py_error.Reset(PyRefType::Borrowed, PyErr_Occurred());
@@ -2779,7 +2779,7 @@ ScriptInterpreterPython::SynchronicityHandler::~SynchronicityHandler() {
 }
 
 bool ScriptInterpreterPython::RunScriptBasedCommand(
-    const char *impl_function, const char *args,
+    const char *impl_function, llvm::StringRef args,
     ScriptedCommandSynchronicity synchronicity,
     lldb_private::CommandReturnObject &cmd_retobj, Status &error,
     const lldb_private::ExecutionContext &exe_ctx) {
@@ -2813,9 +2813,10 @@ bool ScriptInterpreterPython::RunScriptBasedCommand(
 
     SynchronicityHandler synch_handler(debugger_sp, synchronicity);
 
-    ret_val =
-        g_swig_call_command(impl_function, m_dictionary_name.c_str(),
-                            debugger_sp, args, cmd_retobj, exe_ctx_ref_sp);
+    std::string args_str = args.str();
+    ret_val = g_swig_call_command(impl_function, m_dictionary_name.c_str(),
+                                  debugger_sp, args_str.c_str(), cmd_retobj,
+                                  exe_ctx_ref_sp);
   }
 
   if (!ret_val)
@@ -2827,7 +2828,7 @@ bool ScriptInterpreterPython::RunScriptBasedCommand(
 }
 
 bool ScriptInterpreterPython::RunScriptBasedCommand(
-    StructuredData::GenericSP impl_obj_sp, const char *args,
+    StructuredData::GenericSP impl_obj_sp, llvm::StringRef args,
     ScriptedCommandSynchronicity synchronicity,
     lldb_private::CommandReturnObject &cmd_retobj, Status &error,
     const lldb_private::ExecutionContext &exe_ctx) {
@@ -2861,8 +2862,10 @@ bool ScriptInterpreterPython::RunScriptBasedCommand(
 
     SynchronicityHandler synch_handler(debugger_sp, synchronicity);
 
+    std::string args_str = args.str();
     ret_val = g_swig_call_command_object(impl_obj_sp->GetValue(), debugger_sp,
-                                         args, cmd_retobj, exe_ctx_ref_sp);
+                                         args_str.c_str(), cmd_retobj,
+                                         exe_ctx_ref_sp);
   }
 
   if (!ret_val)