lawrence_danna marked 3 inline comments as done.
lawrence_danna added inline comments.
================
Comment at:
lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py:781
+ with open(self.out_filename, 'r') as f:
+ # python2 returns None from write, python3 returns 7
+ lines = [x for x in f.read().strip().split() if x != "7"]
----------------
labath wrote:
> I find this comment confusing. Does that refer to the write call above? If
> so, I don't see how that is relevant here..
When we check the output on the next line we have to strip out the 7, which is
the number of bytes written.
================
Comment at:
lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp:1576-1579
+ // Borrow the FILE*, the lldb_private::File still owns it
+ auto close = [](FILE *f) -> int { fflush(f); return 0; };
+ file_obj =
+ PyFile_FromFile(file.GetStream(), const_cast<char *>(""), cmode, close);
----------------
labath wrote:
> What would you say to passing `fflush` here directly? The signature is
> compatible with fclose, and fclose is documented to return (a superset of)
> the errors also reported by fflush. So, it seems like it might be useful to
> report flushing errors instead of swallowing them. Since accessing the FILE*
> after a fclose call (even if it fails) is UB, and the python code thinks it's
> calling fclose, it shouldn't mess up the FILE state in any way even if the
> flushing fails..
Oh yea good point
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D68737/new/
https://reviews.llvm.org/D68737
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
