labath added inline comments.
================ Comment at: lldb/trunk/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp:1075-1096 +static int readfn(void *ctx, char *buffer, int n) +{ + auto state = PyGILState_Ensure(); + auto *file = (PyObject *) ctx; + int result = -1; + auto pybuffer = PyBuffer_FromMemory(buffer, n); + PyObject *pyresult = NULL; ---------------- lawrence_danna wrote: > zturner wrote: > > I am still pretty unhappy about these functions, and passing function > > pointers into the `File` class. > > > > I think another approach would be this: > > > > 1) Make the `File` class contain a member `std::unique_ptr<IOObject> > > LowLevelIo;` > > > > 2) In `File.cpp`, define something called `class DefaultLowLevelIo : public > > IOObject` that implements the virtual methods against an fd. > > > > 3) In `PythonDataObjects`, define `PythonFileIo : public IOObject` and > > implement the virtual methods against a `PyObject`. > > > > 4) Add an additional constructor to `File` which takes a > > `std::unique_ptr<IOObject> LowLevelIo`, which we can use when creating one > > of these from a python file. > > > > One advantage of this method is that it allows the `PythonFileIo` class to > > be easily tested. > > > > (Also, sorry for not getting back to reviewing this several weeks ago) > I don't see how this approach gets around the problem that the interfaces in > SBDebugger use FILE *, not IOObject > > The only way I can see to do it the way you are saying is to also add a > SBIOObject, with swig wrappers to that, and variants of the SBDebugger > interfaces that take IOObject instead of FILE * > > Do you want to do it that way? What's the final use case here. In the patch itself I don't see anything that would necessitate a FILE * conversion, but I don't know what do you actually intend to use this for. We can always return a null FILE * if the File object is backed by a a python file (we do the same for file descriptors, as there is no way to convert those into FILE*, not without going the fopencookie way). Repository: rL LLVM https://reviews.llvm.org/D38829 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits