[Lldb-commits] [PATCH] D68160: File::Clear() -> File::TakeStreamAndClear()

[Lawrence D'Anna via Phabricator via lldb-commits]

lawrence_danna created this revision.
lawrence_danna added reviewers: JDevlieghere, jasonmolenda, labath.
Herald added a project: LLDB.

File::Clear() is an ugly function.  It's only used in one place,
which is the swig typemaps for FILE*.   This patch refactors and
renames that function to make it clear what it's really for and
why nobody else should use it.

Both File::TakeStreamAndClear() and the FILE* typemaps will be
removed in later patches after a suitable replacement is in place.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D68160

Files:
  lldb/include/lldb/Host/File.h
  lldb/scripts/Python/python-typemaps.swig
  lldb/source/Host/common/File.cpp


Index: lldb/source/Host/common/File.cpp
===================================================================
--- lldb/source/Host/common/File.cpp
+++ lldb/source/Host/common/File.cpp
@@ -162,13 +162,17 @@
   return error;
 }
 
-void File::Clear() {
-  m_stream = nullptr;
+FILE *File::TakeStreamAndClear() {
+  GetStream();
+  FILE *stream = m_stream;
+  m_stream = NULL;
   m_descriptor = kInvalidDescriptor;
   m_options = 0;
   m_own_stream = false;
+  m_own_descriptor = false;
   m_is_interactive = m_supports_colors = m_is_real_terminal =
       eLazyBoolCalculate;
+  return stream;
 }
 
 Status File::GetFileSpec(FileSpec &file_spec) const {
Index: lldb/scripts/Python/python-typemaps.swig
===================================================================
--- lldb/scripts/Python/python-typemaps.swig
+++ lldb/scripts/Python/python-typemaps.swig
@@ -372,6 +372,9 @@
   $1 = $1 || PyCallable_Check(reinterpret_cast<PyObject*>($input));
 }
 
+// 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)
@@ -398,9 +401,7 @@
       lldb::FileUP file = py_file.GetUnderlyingFile();
       if (!file)
          return nullptr;
-      $1 = file->GetStream();
-      if ($1)
-         file->Clear();
+      $1 = file->TakeStreamAndClear();
     }
 }
 
Index: lldb/include/lldb/Host/File.h
===================================================================
--- lldb/include/lldb/Host/File.h
+++ lldb/include/lldb/Host/File.h
@@ -120,7 +120,19 @@
 
   Status Close() override;
 
-  void Clear();
+  /// 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.
+  FILE *TakeStreamAndClear();
 
   int GetDescriptor() const;
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D68160.222247.patch
Type: text/x-patch
Size: 2435 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20190927/81adb587/attachment.bin>