Zachary Ware added the comment: Vajrasky Kok wrote: > However, there are some reviews that I could not implement. > > 1. "This is a good candidate for a custom return converter." > > I can not synchronize struct rlimit and NULL return values.
Looking again, that one is non-trivial, but still doable. You just need a "this means an error happened" value to initialize rl to, and return that value instead of NULL. > 2. "Should be 'class float "PyFloatObject *" "&PyFloat_Type"'. Using > PyFloatObject * instead of PyObject * may require some casts to > PyObject * in some places, but it's better to use the real name." > > I tried it but it was like opening pandora box. It's too much effort > to surpress compile errors and warnings. And casting PyFloatObject > to PyObject in many places, such as functions, macros, makes me > nervous. I think this one deserves a dedicated ticket. It didn't look too bad to me. There are already several places where a value is cast back and forth between PyObject and PyFloatObject. Giving 'self' the right type allows a couple of casts to be removed, and the ones that have to be added are almost exclusively in calls to PyFloat_AsDouble or the CONVERT_TO_DOUBLE macro (which looks like it can just do the casts itself without much issue). I would vote for making PyFloat_AsDouble expect PyFloatObject instead of PyObject, but since (I think?) it looks like it's part of the stable ABI, I'm not sure if that would fly. See http://hg.python.org/sandbox/zware/rev/51473d8c23f8 for a patch on your patch that uses PyFloatObject, compiles cleanly (on win32, at least), and passes relevant tests (though I haven't run the full test suite on this yet; it takes forever on this PC). I have a few more review comments that I hope to get posted later this evening. The patch is looking pretty good overall, though! ---------- _______________________________________ Python tracker <rep...@bugs.python.org> <http://bugs.python.org/issue20185> _______________________________________ _______________________________________________ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
