lawrence_danna created this revision.
lawrence_danna added reviewers: JDevlieghere, jasonmolenda, labath.
Herald added a project: LLDB.
lawrence_danna added parent revisions: D68962: update ScriptInterpreterPython
to use File, not FILE*, D68960: remove FILE* usage from SBStream.i.
The SWIG typemaps for FILE* are no longer used, so
this patch deletes them.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D68963
Files:
lldb/include/lldb/Host/File.h
lldb/scripts/Python/python-typemaps.swig
lldb/source/Host/common/File.cpp
lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
Index: lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
===================================================================
--- lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
+++ lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
@@ -585,8 +585,9 @@
auto file = FileSystem::Instance().Open(FileSpec(FileSystem::DEV_NULL),
File::eOpenOptionRead);
ASSERT_THAT_EXPECTED(file, llvm::Succeeded());
- PythonFile py_file(*file.get(), "r");
- EXPECT_TRUE(PythonFile::Check(py_file.get()));
+ auto py_file = PythonFile::FromFile(*file.get(), "r");
+ ASSERT_THAT_EXPECTED(py_file, llvm::Succeeded());
+ EXPECT_TRUE(PythonFile::Check(py_file.get().get()));
}
TEST_F(PythonDataObjectsTest, TestObjectAttributes) {
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
===================================================================
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -651,10 +651,13 @@
PythonDictionary &sys_module_dict = GetSysModuleDictionary();
+ auto new_file = PythonFile::FromFile(file, mode);
+ if (!new_file)
+ return false;
+
save_file = sys_module_dict.GetItemForKey(PythonString(py_name));
- PythonFile new_file(file, mode);
- sys_module_dict.SetItemForKey(PythonString(py_name), new_file);
+ sys_module_dict.SetItemForKey(PythonString(py_name), new_file.get());
return true;
}
Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
===================================================================
--- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
@@ -637,20 +637,6 @@
static llvm::Expected<PythonFile> FromFile(File &file,
const char *mode = nullptr);
- // FIXME delete this after FILE* typemaps are deleted
- // and ScriptInterpreterPython is fixed
- PythonFile(File &file, const char *mode = nullptr) {
- auto f = FromFile(file, mode);
- if (f)
- *this = std::move(f.get());
- else {
- Reset();
- llvm::consumeError(f.takeError());
- }
- }
-
- lldb::FileUP GetUnderlyingFile() const;
-
llvm::Expected<lldb::FileSP> ConvertToFile(bool borrowed = false);
llvm::Expected<lldb::FileSP>
ConvertToFileForcingUseOfScriptingIOMethods(bool borrowed = false);
Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
===================================================================
--- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
@@ -837,25 +837,6 @@
#endif
}
-FileUP PythonFile::GetUnderlyingFile() const {
- if (!IsValid())
- return nullptr;
-
- // We don't own the file descriptor returned by this function, make sure the
- // File object knows about that.
- PythonString py_mode = GetAttributeValue("mode").AsType<PythonString>();
- auto options = File::GetOptionsFromMode(py_mode.GetString());
- if (!options) {
- llvm::consumeError(options.takeError());
- return nullptr;
- }
- auto file = std::unique_ptr<File>(new NativeFile(
- PyObject_AsFileDescriptor(m_py_obj), options.get(), false));
- if (!file->IsValid())
- return nullptr;
- return file;
-}
-
namespace {
class GIL {
public:
Index: lldb/source/Host/common/File.cpp
===================================================================
--- lldb/source/Host/common/File.cpp
+++ lldb/source/Host/common/File.cpp
@@ -117,8 +117,6 @@
return std::error_code(ENOTSUP, std::system_category());
}
-FILE *File::TakeStreamAndClear() { return nullptr; }
-
int File::GetDescriptor() const { return kInvalidDescriptor; }
FILE *File::GetStream() { return nullptr; }
@@ -331,18 +329,6 @@
return error;
}
-FILE *NativeFile::TakeStreamAndClear() {
- FILE *stream = GetStream();
- m_stream = NULL;
- m_descriptor = kInvalidDescriptor;
- m_options = OpenOptions();
- m_own_stream = false;
- m_own_descriptor = false;
- m_is_interactive = m_supports_colors = m_is_real_terminal =
- eLazyBoolCalculate;
- return stream;
-}
-
Status NativeFile::GetFileSpec(FileSpec &file_spec) const {
Status error;
#ifdef F_GETPATH
Index: lldb/scripts/Python/python-typemaps.swig
===================================================================
--- lldb/scripts/Python/python-typemaps.swig
+++ lldb/scripts/Python/python-typemaps.swig
@@ -451,74 +451,6 @@
}
}
-// FIXME both of these paths wind up calling fdopen() with no provision for ever calling
-// fclose() on the result. SB interfaces that use FILE* should be deprecated for scripting
-// use and this typemap should eventually be removed.
-%typemap(in) FILE * {
- using namespace lldb_private;
- if ($input == Py_None)
- $1 = nullptr;
- else if (!lldb_private::PythonFile::Check($input)) {
- int fd = PyObject_AsFileDescriptor($input);
- if (fd < 0 || PyErr_Occurred())
- return nullptr;
- PythonObject py_input(PyRefType::Borrowed, $input);
- PythonString py_mode = py_input.GetAttributeValue("mode").AsType<PythonString>();
- if (!py_mode.IsValid() || PyErr_Occurred())
- return nullptr;
- FILE *f;
- if ((f = fdopen(fd, py_mode.GetString().str().c_str())))
- $1 = f;
- else {
- PyErr_SetString(PyExc_TypeError, strerror(errno));
- return nullptr;
- }
- }
- else
- {
- PythonFile py_file(PyRefType::Borrowed, $input);
- lldb::FileUP file = py_file.GetUnderlyingFile();
- if (!file)
- return nullptr;
- $1 = file->TakeStreamAndClear();
- }
-}
-
-%typemap(out) FILE * {
- // TODO: This is gross. We should find a way to fetch the mode flags from the
- // lldb_private::File object.
- char mode[4] = {0};
-%#ifdef __APPLE__
- int i = 0;
- if ($1)
- {
- short flags = $1->_flags;
-
- if (flags & __SRD)
- mode[i++] = 'r';
- else if (flags & __SWR)
- mode[i++] = 'w';
- else // if (flags & __SRW)
- mode[i++] = 'a';
- }
-%#else
- // There's no portable way to get the mode here. We just return a mode which
- // permits both reads and writes and count on the operating system to return
- // an error when an invalid operation is attempted.
- mode[0] = 'r';
- mode[1] = '+';
-%#endif
- using namespace lldb_private;
- NativeFile file($1, false);
- PythonFile py_file(file, mode);
- $result = py_file.release();
- if (!$result)
- {
- $result = Py_None;
- Py_INCREF(Py_None);
- }
-}
-
%typemap(in) (const char* string, int len) {
using namespace lldb_private;
if ($input == Py_None)
Index: lldb/include/lldb/Host/File.h
===================================================================
--- lldb/include/lldb/Host/File.h
+++ lldb/include/lldb/Host/File.h
@@ -136,20 +136,6 @@
/// ENOTSUP, success, or another error.
virtual Status GetFileSpec(FileSpec &file_spec) const;
- /// DEPRECATED! Extract the underlying FILE* and reset this File without closing it.
- ///
- /// This is only here to support legacy SB interfaces that need to convert scripting
- /// language objects into FILE* streams. That conversion is inherently sketchy and
- /// doing so may cause the stream to be leaked.
- ///
- /// After calling this the File will be reset to its original state. It will be
- /// invalid and it will not hold on to any resources.
- ///
- /// \return
- /// The underlying FILE* stream from this File, if one exists and can be extracted,
- /// nullptr otherwise.
- virtual FILE *TakeStreamAndClear();
-
/// Get underlying OS file descriptor for this file, or kInvalidDescriptor.
/// If the descriptor is valid, then it may be used directly for I/O
/// However, the File may also perform it's own buffering, so avoid using
@@ -413,7 +399,6 @@
Status Close() override;
WaitableHandle GetWaitableHandle() override;
Status GetFileSpec(FileSpec &file_spec) const override;
- FILE *TakeStreamAndClear() override;
int GetDescriptor() const override;
FILE *GetStream() override;
off_t SeekFromStart(off_t offset, Status *error_ptr = nullptr) override;
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
