Message210168
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! |
|
Date |
User |
Action |
Args |
2014-02-03 22:26:35 | zach.ware | set | recipients:
+ zach.ware, larry, nadeem.vawda, serhiy.storchaka, vajrasky |
2014-02-03 22:26:35 | zach.ware | set | messageid: <1391466395.35.0.827926988796.issue20185@psf.upfronthosting.co.za> |
2014-02-03 22:26:35 | zach.ware | link | issue20185 messages |
2014-02-03 22:26:35 | zach.ware | create | |
|