[Lldb-commits] [lldb] r373285 - File::Clear() -> File::TakeStreamAndClear()

[Lawrence D'Anna via lldb-commits]

Author: lawrence_danna
Date: Mon Sep 30 18:05:02 2019
New Revision: 373285

URL: http://llvm.org/viewvc/llvm-project?rev=373285&view=rev
Log:
File::Clear() -> File::TakeStreamAndClear()

Summary:
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.

Reviewers: JDevlieghere, jasonmolenda, labath

Reviewed By: labath

Subscribers: lldb-commits

Tags: #lldb

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

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

Modified: lldb/trunk/include/lldb/Host/File.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Host/File.h?rev=373285&r1=373284&r2=373285&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Host/File.h (original)
+++ lldb/trunk/include/lldb/Host/File.h Mon Sep 30 18:05:02 2019
@@ -120,7 +120,19 @@ public:
 
   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;
 

Modified: lldb/trunk/scripts/Python/python-typemaps.swig
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/scripts/Python/python-typemaps.swig?rev=373285&r1=373284&r2=373285&view=diff
==============================================================================
--- lldb/trunk/scripts/Python/python-typemaps.swig (original)
+++ lldb/trunk/scripts/Python/python-typemaps.swig Mon Sep 30 18:05:02 2019
@@ -372,6 +372,9 @@ bool SetNumberFromPyObject<double>(doubl
   $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 @@ bool SetNumberFromPyObject<double>(doubl
       lldb::FileUP file = py_file.GetUnderlyingFile();
       if (!file)
          return nullptr;
-      $1 = file->GetStream();
-      if ($1)
-         file->Clear();
+      $1 = file->TakeStreamAndClear();
     }
 }
 

Modified: lldb/trunk/source/Host/common/File.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/common/File.cpp?rev=373285&r1=373284&r2=373285&view=diff
==============================================================================
--- lldb/trunk/source/Host/common/File.cpp (original)
+++ lldb/trunk/source/Host/common/File.cpp Mon Sep 30 18:05:02 2019
@@ -162,13 +162,16 @@ Status File::Close() {
   return error;
 }
 
-void File::Clear() {
-  m_stream = nullptr;
+FILE *File::TakeStreamAndClear() {
+  FILE *stream = GetStream();
+  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 {