This is an archive of the discontinued LLVM Phabricator instance.

No longer pass a StringRef to the Python API
ClosedPublic

Authored by teemperor on Jul 13 2018, 10:53 AM.

Details

Summary

The refactoring patch for DoExecute missed this case of a variadic function that just silently
accepts a StringRef which it then tries to reinterpret as a C-string.

This should fix the Windows builds.

Diff Detail

Repository
rL LLVM

Event Timeline

teemperor created this revision.Jul 13 2018, 10:53 AM

@stella.stamenova I believe this fixes the issue. Thanks for finding this!

This revision is now accepted and ready to land.Jul 13 2018, 10:56 AM
This revision was automatically updated to reflect the committed changes.

All better now! Tests are passing.

If the implementation of a function requires a string - it should probably
accept string (not a StringRef) as a parameter - to avoid back-and-forth
conversions in cases where the caller already has a string to use.

clayborg added inline comments.
lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
860

Of course this could have been fixed by using "s#" which allows the length to be supplied:

Py_BuildValue("(Os#)", session_dict.get(), command.data(), (int)command.size()));