[Lldb-commits] [PATCH] D69488: [LLDB][Python] fix another fflush issue on NetBSD

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rG6a93a12a8dd9: [LLDB][Python] fix another fflush issue on 
NetBSD (authored by lawrence_danna).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69488/new/

https://reviews.llvm.org/D69488

Files:
  lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py
  lldb/source/Host/common/File.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h


Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
===
--- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
@@ -189,6 +189,14 @@
  "key not in dict");
 }
 
+#if PY_MAJOR_VERSION < 3
+// The python 2 API declares some arguments as char* that should
+// be const char *, but it doesn't actually modify them.
+inline char *py2_const_cast(const char *s) { return const_cast(s); }
+#else
+inline const char *py2_const_cast(const char *s) { return s; }
+#endif
+
 enum class PyInitialValue { Invalid, Empty };
 
 template  struct PythonFormat;
@@ -309,16 +317,6 @@
 
   StructuredData::ObjectSP CreateStructuredObject() const;
 
-protected:
-
-#if PY_MAJOR_VERSION < 3
-  // The python 2 API declares some arguments as char* that should
-  // be const char *, but it doesn't actually modify them.
-  static char *py2_const_cast(const char *s) { return const_cast(s); }
-#else
-  static const char *py2_const_cast(const char *s) { return s; }
-#endif
-
 public:
   template 
   llvm::Expected CallMethod(const char *name,
Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
@@ -1502,12 +1502,19 @@
   file_obj = PyFile_FromFd(file.GetDescriptor(), nullptr, mode, -1, nullptr,
"ignore", nullptr, 0);
 #else
-  // Read through the Python source, doesn't seem to modify these strings
-  char *cmode = const_cast(mode);
   // We pass ::flush instead of ::fclose here so we borrow the FILE* --
-  // the lldb_private::File still owns it.
-  file_obj =
-  PyFile_FromFile(file.GetStream(), const_cast(""), cmode, 
::fflush);
+  // the lldb_private::File still owns it.   NetBSD does not allow
+  // input files to be flushed, so we have to check for that case too.
+  int (*closer)(FILE *);
+  auto opts = file.GetOptions();
+  if (!opts)
+return opts.takeError();
+  if (opts.get() & File::eOpenOptionWrite)
+closer = ::fflush;
+  else
+closer = [](FILE *) { return 0; };
+  file_obj = PyFile_FromFile(file.GetStream(), py2_const_cast(""),
+ py2_const_cast(mode), closer);
 #endif
 
   if (!file_obj)
Index: lldb/source/Host/common/File.cpp
===
--- lldb/source/Host/common/File.cpp
+++ lldb/source/Host/common/File.cpp
@@ -310,7 +310,7 @@
 if (m_own_stream) {
   if (::fclose(m_stream) == EOF)
 error.SetErrorToErrno();
-} else {
+} else if (m_options & eOpenOptionWrite) {
   if (::fflush(m_stream) == EOF)
 error.SetErrorToErrno();
 }
Index: 
lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py
===
--- lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py
+++ lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py
@@ -845,11 +845,16 @@
 def i(sbf):
 for i in range(10):
 f = sbf.GetFile()
+self.assertEqual(f.mode, "w")
 yield f
 sbf = lldb.SBFile.Create(f, borrow=True)
 yield sbf
 sbf.Write(str(i).encode('ascii') + b"\n")
 files = list(i(sbf))
+# delete them in reverse order, again because each is a borrow
+# of the previous.
+while files:
+files.pop()
 with open(self.out_filename, 'r') as f:
 self.assertEqual(list(range(10)), list(map(int, 
f.read().strip().split(
 


Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
===
--- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
@@ -189,6 +189,14 @@
  "key not in dict");
 }
 
+#if PY_MAJOR_VERSION < 3
+// The python 2 API declares some arguments as char* that should
+// be const 

[Lldb-commits] [PATCH] D69488: [LLDB][Python] fix another fflush issue on NetBSD

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added a comment.

Ok, let's get this thing fixed. We can continue the discussion on the other 
patch.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69488/new/

https://reviews.llvm.org/D69488



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D69488: [LLDB][Python] fix another fflush issue on NetBSD

[Michał Górny via Phabricator via lldb-commits]

mgorny accepted this revision.
mgorny added a comment.
This revision is now accepted and ready to land.

WFM. Thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69488/new/

https://reviews.llvm.org/D69488



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D69488: [LLDB][Python] fix another fflush issue on NetBSD

[Michał Górny via Phabricator via lldb-commits]

mgorny added a comment.

Diff 4, I think. Lemme do a quick test with 5. Hopefully it won't request me to 
rebuild everything again.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69488/new/

https://reviews.llvm.org/D69488



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D69488: [LLDB][Python] fix another fflush issue on NetBSD

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

lawrence_danna updated this revision to Diff 226747.
lawrence_danna added a comment.

revert to previous version


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69488/new/

https://reviews.llvm.org/D69488

Files:
  lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py
  lldb/source/Host/common/File.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h


Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
===
--- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
@@ -189,6 +189,14 @@
  "key not in dict");
 }
 
+#if PY_MAJOR_VERSION < 3
+// The python 2 API declares some arguments as char* that should
+// be const char *, but it doesn't actually modify them.
+inline char *py2_const_cast(const char *s) { return const_cast(s); }
+#else
+inline const char *py2_const_cast(const char *s) { return s; }
+#endif
+
 enum class PyInitialValue { Invalid, Empty };
 
 template  struct PythonFormat;
@@ -309,16 +317,6 @@
 
   StructuredData::ObjectSP CreateStructuredObject() const;
 
-protected:
-
-#if PY_MAJOR_VERSION < 3
-  // The python 2 API declares some arguments as char* that should
-  // be const char *, but it doesn't actually modify them.
-  static char *py2_const_cast(const char *s) { return const_cast(s); }
-#else
-  static const char *py2_const_cast(const char *s) { return s; }
-#endif
-
 public:
   template 
   llvm::Expected CallMethod(const char *name,
Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
@@ -1502,12 +1502,19 @@
   file_obj = PyFile_FromFd(file.GetDescriptor(), nullptr, mode, -1, nullptr,
"ignore", nullptr, 0);
 #else
-  // Read through the Python source, doesn't seem to modify these strings
-  char *cmode = const_cast(mode);
   // We pass ::flush instead of ::fclose here so we borrow the FILE* --
-  // the lldb_private::File still owns it.
-  file_obj =
-  PyFile_FromFile(file.GetStream(), const_cast(""), cmode, 
::fflush);
+  // the lldb_private::File still owns it.   NetBSD does not allow
+  // input files to be flushed, so we have to check for that case too.
+  int (*closer)(FILE *);
+  auto opts = file.GetOptions();
+  if (!opts)
+return opts.takeError();
+  if (opts.get() & File::eOpenOptionWrite)
+closer = ::fflush;
+  else
+closer = [](FILE *) { return 0; };
+  file_obj = PyFile_FromFile(file.GetStream(), py2_const_cast(""),
+ py2_const_cast(mode), closer);
 #endif
 
   if (!file_obj)
Index: lldb/source/Host/common/File.cpp
===
--- lldb/source/Host/common/File.cpp
+++ lldb/source/Host/common/File.cpp
@@ -310,7 +310,7 @@
 if (m_own_stream) {
   if (::fclose(m_stream) == EOF)
 error.SetErrorToErrno();
-} else {
+} else if (m_options & eOpenOptionWrite) {
   if (::fflush(m_stream) == EOF)
 error.SetErrorToErrno();
 }
Index: 
lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py
===
--- lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py
+++ lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py
@@ -845,11 +845,16 @@
 def i(sbf):
 for i in range(10):
 f = sbf.GetFile()
+self.assertEqual(f.mode, "w")
 yield f
 sbf = lldb.SBFile.Create(f, borrow=True)
 yield sbf
 sbf.Write(str(i).encode('ascii') + b"\n")
 files = list(i(sbf))
+# delete them in reverse order, again because each is a borrow
+# of the previous.
+while files:
+files.pop()
 with open(self.out_filename, 'r') as f:
 self.assertEqual(list(range(10)), list(map(int, 
f.read().strip().split(
 


Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
===
--- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
@@ -189,6 +189,14 @@
  "key not in dict");
 }
 
+#if PY_MAJOR_VERSION < 3
+// The python 2 API declares some arguments as char* that should
+// be const char *, but it doesn't actually modify them.
+inline char 

[Lldb-commits] [PATCH] D69488: [LLDB][Python] fix another fflush issue on NetBSD

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

lawrence_danna added a comment.

In D69488#1724178 , @mgorny wrote:

> I have confirmed that your previous revision fixed the problem in question. 
> The newer one would probably require full test suite run which I can't do 
> right now. As I said, I would prefer those two changes to be committed 
> separately, preferably with a few hours delay so that it would be clear if it 
> causes any breakage on buildbot.


which commit id did you test?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69488/new/

https://reviews.llvm.org/D69488



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D69488: [LLDB][Python] fix another fflush issue on NetBSD

[Michał Górny via Phabricator via lldb-commits]

mgorny added a comment.

I have confirmed that your previous revision fixed the problem in question. The 
newer one would probably require full test suite run which I can't do right 
now. As I said, I would prefer those two changes to be committed separately, 
preferably with a few hours delay so that it would be clear if it causes any 
breakage on buildbot.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69488/new/

https://reviews.llvm.org/D69488



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D69488: [LLDB][Python] fix another fflush issue on NetBSD

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

lawrence_danna added a comment.

In D69488#1723481 , @labath wrote:

> I am not thrilled by all of that duping going around. Having multiple FILE 
> objects means that you have multiple independent file caches too. That can 
> cause different kinds of strange behavior if multiple things start 
> reading/writing to the different FILE objects simultaneously. I think I'd 
> rather just keep the existing borrow semantics. I don't think that should be 
> a problem in practice -- it's just that this test is weird because is testing 
> the extreme cases of this behavior (which is fine). Normally you'll just use 
> one level of wrapping and so the underlying file will be naturally kept 
> around, since lldb will still be holding onto it.


I'm not exactly thrilled with it but I will still argue it is the right thing 
to do.   If you look at the docstring for `GetFile`, it doesn't mention this 
problem.   It says you can't use a borrowed file if it becomes dangling, but it 
doesn't say you have to delete your references to it before it can dangle.   
That's because this problem is surprising enough that neither of us thought of 
it.

Consider this situation:   The python programmer makes an object that keeps a 
reference to `debugger` and also to `debugger.GetOutputFile().GetFile()`.   
When they're done with the object they destroy the debugger and let their 
object go out of scope.

In python3, they may get an `IOError` at this point, as the saved output file 
tries to flush its buffer to a closed file.In python 2 they are exposed to 
to a use-after free violation and the program could crash!

It is very difficult to write  python programs that deal with these kind of 
semantics correctly, and nothing about this API suggests that it is as 
dangerous to use as that.  I think the correctness and ergonomics concerns 
outweigh the performance concerns here. Particularly because this issue 
only affects python 2. Is it really a bigger deal that we generate some 
extra dups on an end-of-life scripting language than that we potentially crash 
on that language?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69488/new/

https://reviews.llvm.org/D69488



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D69488: [LLDB][Python] fix another fflush issue on NetBSD

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I am not thrilled by all of that duping going around. Having multiple FILE 
objects means that you have multiple independent file caches too. That can 
cause different kinds of strange behavior if multiple things start 
reading/writing to the different FILE objects simultaneously. I think I'd 
rather just keep the existing borrow semantics. I don't think that should be a 
problem in practice -- it's just that this test is weird because is testing the 
extreme cases of this behavior (which is fine). Normally you'll just use one 
level of wrapping and so the underlying file will be naturally kept around, 
since lldb will still be holding onto it.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69488/new/

https://reviews.llvm.org/D69488



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D69488: [LLDB][Python] fix another fflush issue on NetBSD

[Michał Górny via Phabricator via lldb-commits]

mgorny added a comment.

Thanks but I'd personally prefer splitting this into a future commit, in case 
it introduced unrelated issues.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69488/new/

https://reviews.llvm.org/D69488



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D69488: [LLDB][Python] fix another fflush issue on NetBSD

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

lawrence_danna added a comment.

There, I think that fixes the issue with borrow semantics and the NetBSD issues.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69488/new/

https://reviews.llvm.org/D69488



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D69488: [LLDB][Python] fix another fflush issue on NetBSD

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

lawrence_danna updated this revision to Diff 226618.
lawrence_danna added a comment.

protect python from being exposed to C++ reference borrowing semantics


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69488/new/

https://reviews.llvm.org/D69488

Files:
  lldb/include/lldb/Host/File.h
  lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py
  lldb/source/Host/common/File.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h

Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
===
--- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
@@ -189,6 +189,14 @@
  "key not in dict");
 }
 
+#if PY_MAJOR_VERSION < 3
+// The python 2 API declares some arguments as char* that should
+// be const char *, but it doesn't actually modify them.
+inline char *py2_const_cast(const char *s) { return const_cast(s); }
+#else
+inline const char *py2_const_cast(const char *s) { return s; }
+#endif
+
 enum class PyInitialValue { Invalid, Empty };
 
 template  struct PythonFormat;
@@ -309,16 +317,6 @@
 
   StructuredData::ObjectSP CreateStructuredObject() const;
 
-protected:
-
-#if PY_MAJOR_VERSION < 3
-  // The python 2 API declares some arguments as char* that should
-  // be const char *, but it doesn't actually modify them.
-  static char *py2_const_cast(const char *s) { return const_cast(s); }
-#else
-  static const char *py2_const_cast(const char *s) { return s; }
-#endif
-
 public:
   template 
   llvm::Expected CallMethod(const char *name,
Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
@@ -1502,12 +1502,28 @@
   file_obj = PyFile_FromFd(file.GetDescriptor(), nullptr, mode, -1, nullptr,
"ignore", nullptr, 0);
 #else
-  // Read through the Python source, doesn't seem to modify these strings
-  char *cmode = const_cast(mode);
-  // We pass ::flush instead of ::fclose here so we borrow the FILE* --
-  // the lldb_private::File still owns it.
-  file_obj =
-  PyFile_FromFile(file.GetStream(), const_cast(""), cmode, ::fflush);
+  // It's more or less safe to let a python program borrow a file descriptor.
+  // If they let it dangle and then use it, they'll probably get an exception.
+  // The worst that can happen is they'll wind up doing IO on the wrong
+  // descriptor.   But it would be very unsafe to let a python program borrow
+  // a FILE*.   They could actually crash the program just by keeping a
+  // reference to it around.Since in python 2 we must have a FILE* and
+  // not a descriptor, we dup the descriptor and fdopen a new FILE* to it
+  // so python can have something it can own safely.
+  auto opts = File::GetOptionsFromMode(mode);
+  if (!opts)
+return opts.takeError();
+  int fd = file.GetDescriptor();
+  if (!File::DescriptorIsValid(fd))
+return llvm::createStringError(llvm::inconvertibleErrorCode(),
+   "File has no file descriptor");
+  NativeFile dupfile(fd, opts.get(), false);
+  FILE *stream = NativeFile::ReleaseFILE(std::move(dupfile));
+  if (!stream)
+return llvm::createStringError(llvm::inconvertibleErrorCode(),
+   "could not create FILE* stream");
+  file_obj = PyFile_FromFile(stream, py2_const_cast(""), py2_const_cast(mode),
+ ::fclose);
 #endif
 
   if (!file_obj)
Index: lldb/source/Host/common/File.cpp
===
--- lldb/source/Host/common/File.cpp
+++ lldb/source/Host/common/File.cpp
@@ -304,13 +304,25 @@
   return m_stream;
 }
 
+FILE *NativeFile::ReleaseFILE(NativeFile &) {
+  FILE *stream = nullptr;
+  file.GetStream();
+  if (file.m_own_stream) {
+stream = file.m_stream;
+file.m_own_stream = false;
+file.m_stream = nullptr;
+  }
+  file.Close();
+  return stream;
+}
+
 Status NativeFile::Close() {
   Status error;
   if (StreamIsValid()) {
 if (m_own_stream) {
   if (::fclose(m_stream) == EOF)
 error.SetErrorToErrno();
-} else {
+} else if (m_options & eOpenOptionWrite) {
   if (::fflush(m_stream) == EOF)
 error.SetErrorToErrno();
 }
Index: lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py
===
--- lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py
+++ lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py
@@ -845,6 +845,7 @@
   

[Lldb-commits] [PATCH] D69488: [LLDB][Python] fix another fflush issue on NetBSD

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

lawrence_danna marked an inline comment as done.
lawrence_danna added inline comments.



Comment at: 
lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py:854
 files = list(i(sbf))
+# delete them in reverse order, again because each is a borrow
+# of the previous.

lawrence_danna wrote:
> mgorny wrote:
> > For the record, this doesn't really guarantee specific order of 
> > destruction. Generally, in Python code you shouldn't rely on destructors 
> > being called at all and close files manually. That's why `with` is commonly 
> > used with files.
> The point is not to rely on the file being closed at a certain time.   They 
> point is that each file added to the list is a borrowed reference to the 
> previous one, and we should not allow those references to become dangling by 
> letting the older ones go to zero-references before the younger ones.
> 
> Now that I say it, I wonder if it was a bad idea to expose this kind of C++ 
> object lifetime semantics to python programs.
> I put in `GetFile` mainly so that in the case that a SBFile was a proxy of a 
> python file, you could get the python file back.
> 
> It might be better to restrict it to that case, and return None instead of a 
> borrowed file in other cases.
> 
> On the other hand, there's value in having a consistent API.  Under the 
> current semantics this just works:
> 
> ```
> print("foo",   file=debugger.GetOutputFile().GetFile())
> ```
> 
> Overall I think I'd lean towards keeping GetFile the way it is, but I can see 
> the argument for restricting it.
On second second thought, there's a better way.   We don't need to expose the 
borrow semantics, and we don't need to restrict GetFile like that.  We can 
do the same thing File.cpp does when you ask it for a FILE* on a borrowed FD -- 
that is, use dup and create one that python can truly own.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69488/new/

https://reviews.llvm.org/D69488



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D69488: [LLDB][Python] fix another fflush issue on NetBSD

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

lawrence_danna marked 2 inline comments as done.
lawrence_danna added inline comments.



Comment at: 
lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py:854
 files = list(i(sbf))
+# delete them in reverse order, again because each is a borrow
+# of the previous.

mgorny wrote:
> For the record, this doesn't really guarantee specific order of destruction. 
> Generally, in Python code you shouldn't rely on destructors being called at 
> all and close files manually. That's why `with` is commonly used with files.
The point is not to rely on the file being closed at a certain time.   They 
point is that each file added to the list is a borrowed reference to the 
previous one, and we should not allow those references to become dangling by 
letting the older ones go to zero-references before the younger ones.

Now that I say it, I wonder if it was a bad idea to expose this kind of C++ 
object lifetime semantics to python programs.
I put in `GetFile` mainly so that in the case that a SBFile was a proxy of a 
python file, you could get the python file back.

It might be better to restrict it to that case, and return None instead of a 
borrowed file in other cases.

On the other hand, there's value in having a consistent API.  Under the current 
semantics this just works:

```
print("foo",   file=debugger.GetOutputFile().GetFile())
```

Overall I think I'd lean towards keeping GetFile the way it is, but I can see 
the argument for restricting it.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69488/new/

https://reviews.llvm.org/D69488



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D69488: [LLDB][Python] fix another fflush issue on NetBSD

[Michał Górny via Phabricator via lldb-commits]

mgorny added a comment.

I'm sorry but I won't be able to test it until later today. Visually, looks 
good though.




Comment at: 
lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py:854
 files = list(i(sbf))
+# delete them in reverse order, again because each is a borrow
+# of the previous.

For the record, this doesn't really guarantee specific order of destruction. 
Generally, in Python code you shouldn't rely on destructors being called at all 
and close files manually. That's why `with` is commonly used with files.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69488/new/

https://reviews.llvm.org/D69488



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D69488: [LLDB][Python] fix another fflush issue on NetBSD

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

lawrence_danna updated this revision to Diff 226599.
lawrence_danna added a comment.

and another


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69488/new/

https://reviews.llvm.org/D69488

Files:
  lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py
  lldb/source/Host/common/File.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h


Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
===
--- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
@@ -189,6 +189,14 @@
  "key not in dict");
 }
 
+#if PY_MAJOR_VERSION < 3
+// The python 2 API declares some arguments as char* that should
+// be const char *, but it doesn't actually modify them.
+inline char *py2_const_cast(const char *s) { return const_cast(s); }
+#else
+inline const char *py2_const_cast(const char *s) { return s; }
+#endif
+
 enum class PyInitialValue { Invalid, Empty };
 
 template  struct PythonFormat;
@@ -309,16 +317,6 @@
 
   StructuredData::ObjectSP CreateStructuredObject() const;
 
-protected:
-
-#if PY_MAJOR_VERSION < 3
-  // The python 2 API declares some arguments as char* that should
-  // be const char *, but it doesn't actually modify them.
-  static char *py2_const_cast(const char *s) { return const_cast(s); }
-#else
-  static const char *py2_const_cast(const char *s) { return s; }
-#endif
-
 public:
   template 
   llvm::Expected CallMethod(const char *name,
Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
@@ -1502,12 +1502,19 @@
   file_obj = PyFile_FromFd(file.GetDescriptor(), nullptr, mode, -1, nullptr,
"ignore", nullptr, 0);
 #else
-  // Read through the Python source, doesn't seem to modify these strings
-  char *cmode = const_cast(mode);
   // We pass ::flush instead of ::fclose here so we borrow the FILE* --
-  // the lldb_private::File still owns it.
-  file_obj =
-  PyFile_FromFile(file.GetStream(), const_cast(""), cmode, 
::fflush);
+  // the lldb_private::File still owns it.   NetBSD does not allow
+  // input files to be flushed, so we have to check for that case too.
+  int (*closer)(FILE *);
+  auto opts = file.GetOptions();
+  if (!opts)
+return opts.takeError();
+  if (opts.get() & File::eOpenOptionWrite)
+closer = ::fflush;
+  else
+closer = [](FILE *) { return 0; };
+  file_obj = PyFile_FromFile(file.GetStream(), py2_const_cast(""),
+ py2_const_cast(mode), closer);
 #endif
 
   if (!file_obj)
Index: lldb/source/Host/common/File.cpp
===
--- lldb/source/Host/common/File.cpp
+++ lldb/source/Host/common/File.cpp
@@ -310,7 +310,7 @@
 if (m_own_stream) {
   if (::fclose(m_stream) == EOF)
 error.SetErrorToErrno();
-} else {
+} else if (m_options & eOpenOptionWrite) {
   if (::fflush(m_stream) == EOF)
 error.SetErrorToErrno();
 }
Index: 
lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py
===
--- lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py
+++ lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py
@@ -845,11 +845,16 @@
 def i(sbf):
 for i in range(10):
 f = sbf.GetFile()
+self.assertEqual(f.mode, "w")
 yield f
 sbf = lldb.SBFile.Create(f, borrow=True)
 yield sbf
 sbf.Write(str(i).encode('ascii') + b"\n")
 files = list(i(sbf))
+# delete them in reverse order, again because each is a borrow
+# of the previous.
+while files:
+files.pop()
 with open(self.out_filename, 'r') as f:
 self.assertEqual(list(range(10)), list(map(int, 
f.read().strip().split(
 


Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
===
--- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
@@ -189,6 +189,14 @@
  "key not in dict");
 }
 
+#if PY_MAJOR_VERSION < 3
+// The python 2 API declares some arguments as char* that should
+// be const char *, but it doesn't actually modify them.
+inline char *py2_const_cast(const char *s) 

[Lldb-commits] [PATCH] D69488: [LLDB][Python] fix another fflush issue on NetBSD

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

lawrence_danna updated this revision to Diff 226597.
lawrence_danna added a comment.

found another one


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69488/new/

https://reviews.llvm.org/D69488

Files:
  lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py
  lldb/source/Host/common/File.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h


Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
===
--- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
@@ -189,6 +189,14 @@
  "key not in dict");
 }
 
+#if PY_MAJOR_VERSION < 3
+// The python 2 API declares some arguments as char* that should
+// be const char *, but it doesn't actually modify them.
+inline char *py2_const_cast(const char *s) { return const_cast(s); }
+#else
+inline const char *py2_const_cast(const char *s) { return s; }
+#endif
+
 enum class PyInitialValue { Invalid, Empty };
 
 template  struct PythonFormat;
@@ -309,16 +317,6 @@
 
   StructuredData::ObjectSP CreateStructuredObject() const;
 
-protected:
-
-#if PY_MAJOR_VERSION < 3
-  // The python 2 API declares some arguments as char* that should
-  // be const char *, but it doesn't actually modify them.
-  static char *py2_const_cast(const char *s) { return const_cast(s); }
-#else
-  static const char *py2_const_cast(const char *s) { return s; }
-#endif
-
 public:
   template 
   llvm::Expected CallMethod(const char *name,
Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
@@ -1502,12 +1502,19 @@
   file_obj = PyFile_FromFd(file.GetDescriptor(), nullptr, mode, -1, nullptr,
"ignore", nullptr, 0);
 #else
-  // Read through the Python source, doesn't seem to modify these strings
-  char *cmode = const_cast(mode);
   // We pass ::flush instead of ::fclose here so we borrow the FILE* --
-  // the lldb_private::File still owns it.
-  file_obj =
-  PyFile_FromFile(file.GetStream(), const_cast(""), cmode, 
::fflush);
+  // the lldb_private::File still owns it.   NetBSD does not allow
+  // input files to be flushed, so we have to check for that case too.
+  int (*closer)(FILE *);
+  auto opts = file.GetOptions();
+  if (!opts)
+return opts.takeError();
+  if (opts.get() & File::eOpenOptionWrite)
+closer = ::fflush;
+  else
+closer = [](FILE *) { return 0; };
+  file_obj = PyFile_FromFile(file.GetStream(), py2_const_cast(""),
+ py2_const_cast(mode), closer);
 #endif
 
   if (!file_obj)
Index: lldb/source/Host/common/File.cpp
===
--- lldb/source/Host/common/File.cpp
+++ lldb/source/Host/common/File.cpp
@@ -310,7 +310,7 @@
 if (m_own_stream) {
   if (::fclose(m_stream) == EOF)
 error.SetErrorToErrno();
-} else {
+} else if (m_options & eOpenOptionWrite) {
   if (::fflush(m_stream) == EOF)
 error.SetErrorToErrno();
 }
Index: 
lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py
===
--- lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py
+++ lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py
@@ -845,6 +845,7 @@
 def i(sbf):
 for i in range(10):
 f = sbf.GetFile()
+self.assertEqual(f.mode, "w")
 yield f
 sbf = lldb.SBFile.Create(f, borrow=True)
 yield sbf


Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
===
--- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
@@ -189,6 +189,14 @@
  "key not in dict");
 }
 
+#if PY_MAJOR_VERSION < 3
+// The python 2 API declares some arguments as char* that should
+// be const char *, but it doesn't actually modify them.
+inline char *py2_const_cast(const char *s) { return const_cast(s); }
+#else
+inline const char *py2_const_cast(const char *s) { return s; }
+#endif
+
 enum class PyInitialValue { Invalid, Empty };
 
 template  struct PythonFormat;
@@ -309,16 +317,6 @@
 
   StructuredData::ObjectSP CreateStructuredObject() const;
 
-protected:
-
-#if PY_MAJOR_VERSION < 3
-  // The python 2 API declares some arguments as char* that should
-  // be const 

[Lldb-commits] [PATCH] D69488: [LLDB][Python] fix another fflush issue on NetBSD

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

lawrence_danna updated this revision to Diff 226595.
lawrence_danna added a comment.

py2_const_cast shouldn't be static as a free function


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69488/new/

https://reviews.llvm.org/D69488

Files:
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h


Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
===
--- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
@@ -189,6 +189,14 @@
  "key not in dict");
 }
 
+#if PY_MAJOR_VERSION < 3
+// The python 2 API declares some arguments as char* that should
+// be const char *, but it doesn't actually modify them.
+char *py2_const_cast(const char *s) { return const_cast(s); }
+#else
+const char *py2_const_cast(const char *s) { return s; }
+#endif
+
 enum class PyInitialValue { Invalid, Empty };
 
 template  struct PythonFormat;
@@ -309,16 +317,6 @@
 
   StructuredData::ObjectSP CreateStructuredObject() const;
 
-protected:
-
-#if PY_MAJOR_VERSION < 3
-  // The python 2 API declares some arguments as char* that should
-  // be const char *, but it doesn't actually modify them.
-  static char *py2_const_cast(const char *s) { return const_cast(s); }
-#else
-  static const char *py2_const_cast(const char *s) { return s; }
-#endif
-
 public:
   template 
   llvm::Expected CallMethod(const char *name,
Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
@@ -1502,12 +1502,19 @@
   file_obj = PyFile_FromFd(file.GetDescriptor(), nullptr, mode, -1, nullptr,
"ignore", nullptr, 0);
 #else
-  // Read through the Python source, doesn't seem to modify these strings
-  char *cmode = const_cast(mode);
   // We pass ::flush instead of ::fclose here so we borrow the FILE* --
-  // the lldb_private::File still owns it.
-  file_obj =
-  PyFile_FromFile(file.GetStream(), const_cast(""), cmode, 
::fflush);
+  // the lldb_private::File still owns it.   NetBSD does not allow
+  // input files to be flushed, so we have to check for that case too.
+  int (*closer)(FILE *);
+  auto opts = file.GetOptions();
+  if (!opts)
+return opts.takeError();
+  if (opts.get() & File::eOpenOptionWrite)
+closer = ::fflush;
+  else
+closer = [](FILE *) { return 0; };
+  file_obj = PyFile_FromFile(file.GetStream(), py2_const_cast(""),
+ py2_const_cast(mode), closer);
 #endif
 
   if (!file_obj)


Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
===
--- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
@@ -189,6 +189,14 @@
  "key not in dict");
 }
 
+#if PY_MAJOR_VERSION < 3
+// The python 2 API declares some arguments as char* that should
+// be const char *, but it doesn't actually modify them.
+char *py2_const_cast(const char *s) { return const_cast(s); }
+#else
+const char *py2_const_cast(const char *s) { return s; }
+#endif
+
 enum class PyInitialValue { Invalid, Empty };
 
 template  struct PythonFormat;
@@ -309,16 +317,6 @@
 
   StructuredData::ObjectSP CreateStructuredObject() const;
 
-protected:
-
-#if PY_MAJOR_VERSION < 3
-  // The python 2 API declares some arguments as char* that should
-  // be const char *, but it doesn't actually modify them.
-  static char *py2_const_cast(const char *s) { return const_cast(s); }
-#else
-  static const char *py2_const_cast(const char *s) { return s; }
-#endif
-
 public:
   template 
   llvm::Expected CallMethod(const char *name,
Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
@@ -1502,12 +1502,19 @@
   file_obj = PyFile_FromFd(file.GetDescriptor(), nullptr, mode, -1, nullptr,
"ignore", nullptr, 0);
 #else
-  // Read through the Python source, doesn't seem to modify these strings
-  char *cmode = const_cast(mode);
   // We pass ::flush instead of ::fclose here so we borrow the FILE* --
-  // the lldb_private::File still owns it.
-  file_obj =
-  PyFile_FromFile(file.GetStream(), const_cast(""), cmode, ::fflush);
+  // the lldb_private::File still owns it.   NetBSD does not allow
+  // input files to be flushed, so we have to check for 

[Lldb-commits] [PATCH] D69488: [LLDB][Python] fix another fflush issue on NetBSD

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

lawrence_danna created this revision.
lawrence_danna added reviewers: labath, mgorny.
Herald added a subscriber: krytarowski.
Herald added a project: LLDB.

Here's another instance where we were calling fflush on an input 
stream, which is illegal on NetBSD.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D69488

Files:
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h


Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
===
--- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
@@ -189,6 +189,14 @@
  "key not in dict");
 }
 
+#if PY_MAJOR_VERSION < 3
+// The python 2 API declares some arguments as char* that should
+// be const char *, but it doesn't actually modify them.
+static char *py2_const_cast(const char *s) { return const_cast(s); }
+#else
+static const char *py2_const_cast(const char *s) { return s; }
+#endif
+
 enum class PyInitialValue { Invalid, Empty };
 
 template  struct PythonFormat;
@@ -309,16 +317,6 @@
 
   StructuredData::ObjectSP CreateStructuredObject() const;
 
-protected:
-
-#if PY_MAJOR_VERSION < 3
-  // The python 2 API declares some arguments as char* that should
-  // be const char *, but it doesn't actually modify them.
-  static char *py2_const_cast(const char *s) { return const_cast(s); }
-#else
-  static const char *py2_const_cast(const char *s) { return s; }
-#endif
-
 public:
   template 
   llvm::Expected CallMethod(const char *name,
Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
@@ -1502,12 +1502,19 @@
   file_obj = PyFile_FromFd(file.GetDescriptor(), nullptr, mode, -1, nullptr,
"ignore", nullptr, 0);
 #else
-  // Read through the Python source, doesn't seem to modify these strings
-  char *cmode = const_cast(mode);
   // We pass ::flush instead of ::fclose here so we borrow the FILE* --
-  // the lldb_private::File still owns it.
-  file_obj =
-  PyFile_FromFile(file.GetStream(), const_cast(""), cmode, 
::fflush);
+  // the lldb_private::File still owns it.   NetBSD does not allow
+  // input files to be flushed, so we have to check for that case too.
+  int (*closer)(FILE *);
+  auto opts = file.GetOptions();
+  if (!opts)
+return opts.takeError();
+  if (opts.get() & File::eOpenOptionWrite)
+closer = ::fflush;
+  else
+closer = [](FILE *) { return 0; };
+  file_obj = PyFile_FromFile(file.GetStream(), py2_const_cast(""),
+ py2_const_cast(mode), closer);
 #endif
 
   if (!file_obj)


Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
===
--- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
@@ -189,6 +189,14 @@
  "key not in dict");
 }
 
+#if PY_MAJOR_VERSION < 3
+// The python 2 API declares some arguments as char* that should
+// be const char *, but it doesn't actually modify them.
+static char *py2_const_cast(const char *s) { return const_cast(s); }
+#else
+static const char *py2_const_cast(const char *s) { return s; }
+#endif
+
 enum class PyInitialValue { Invalid, Empty };
 
 template  struct PythonFormat;
@@ -309,16 +317,6 @@
 
   StructuredData::ObjectSP CreateStructuredObject() const;
 
-protected:
-
-#if PY_MAJOR_VERSION < 3
-  // The python 2 API declares some arguments as char* that should
-  // be const char *, but it doesn't actually modify them.
-  static char *py2_const_cast(const char *s) { return const_cast(s); }
-#else
-  static const char *py2_const_cast(const char *s) { return s; }
-#endif
-
 public:
   template 
   llvm::Expected CallMethod(const char *name,
Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
@@ -1502,12 +1502,19 @@
   file_obj = PyFile_FromFd(file.GetDescriptor(), nullptr, mode, -1, nullptr,
"ignore", nullptr, 0);
 #else
-  // Read through the Python source, doesn't seem to modify these strings
-  char *cmode = const_cast(mode);
   // We pass ::flush instead of ::fclose here so we borrow the FILE* --
-  // the lldb_private::File still owns it.
-  file_obj =
-  PyFile_FromFile(file.GetStream(), const_cast(""), cmode, ::fflush);
+  // the lldb_private::File still owns it.