[Python] Update PyString_FromString() to work for python 2 and 3.

[JDevlieghere]

There's two more mentions of this function:

• unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
• source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp

I guess these have to be updated as well? Would it be worth to define a PYSTRING_FROM_STRING macro?

[zturner]

In D56511#1351768, @JDevlieghere wrote:

There's two more mentions of this function:

• unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
• source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp

I guess these have to be updated as well? Would it be worth to define a PYSTRING_FROM_STRING macro?

I think it's the other way around. Definitely the one in PythonDataObjects.cpp should remain, and the code in this patch should be updated to use PythonString, which already abstracts all this away. In general we should not need explicit version checks anywhere outside of PythonDataObjects.cpp, as the whole purpose of that is to abstract away the differences so we don't need ifdefs in our code.

lldb/scripts/Python/python-swigsafecast.swig

28–39 ↗

(On Diff #180912)

This entire function can just be deleted, it's not used.

lldb/scripts/Python/python-wrapper.swig

829–835 ↗

(On Diff #180912)

Instead of the version checks, use this:

PythonString str(callee_name);
PyObject *result = PyObject_CallMethodObjectArgs(implementor, str.GetObject(), arg, nullptr);

[zturner]

lldb/scripts/Python/python-wrapper.swig

834 ↗

(On Diff #180912)

Note that there appears to be an object leak here, as the str_arg is never released. Changing to PythonString should also fix that bug as a free side effect.

[davide]

Address comments

[zturner]

lldb/scripts/Python/python-wrapper.swig

827 ↗

(On Diff #180924)

This looks like another possible object leak. The documentation for PyObject_CallMethodObjArgs doesn't mention whether it takes ownership of the argument, but I think it wouldn't make sense for it to do so, because you're just calling a function. In any case, just make sure the current change doesn't regress the test suite.

lldb/trunk/scripts/Python/python-swigsafecast.swig

 PyObject*
 SBTypeToSWIGWrapper (PyObject* py_object)
 {
     return py_object;
 }
 
 template <>
 PyObject*
-SBTypeToSWIGWrapper (const char* c_str)
-{
-    if (c_str)
-        return PyString_FromString(c_str);
-    return NULL;
-}
-
-template <>
-PyObject*
 SBTypeToSWIGWrapper (unsigned int* c_int)
 {
     if (!c_int)
         return NULL;
     return PyInt_FromLong(*c_int);
 }
 
 template <>

lldb/trunk/scripts/Python/python-wrapper.swig

 {
     using namespace lldb_private;
 
     static char callee_name[] = "get_recognized_arguments";
 
     lldb::SBFrame frame_sb(frame_sp);
     PyObject *arg = SBTypeToSWIGWrapper(frame_sb);
 
-    PyObject* result = PyObject_CallMethodObjArgs(implementor, PyString_FromString(callee_name), arg, NULL);
+    PythonString str(callee_name);
+    PyObject* result = PyObject_CallMethodObjArgs(implementor, str.get(), arg,
+                                                  NULL);
     return result;
 }
 
 SWIGEXPORT void*
 LLDBSWIGPython_GetDynamicSetting (void* module, const char* setting, const lldb::TargetSP& target_sp)
 {
     using namespace lldb_private;