MAINT: Inline gentype_getreadbuf #15422
Conversation
There was a problem hiding this comment.
I am OK with just putting it in as is but if you have time, happy to reduce things a bit more even. I am curious if we can somehow remove that whole Malloc+free block, since we are handling a new object we are creating anyway, it is fine to byteswap in-place. But I am a bit worried of missing a corner case.
| PyArray_Descr *outcode = PyArray_DescrFromScalar(self); | ||
| data = (void *)scalar_value(self, outcode); | ||
| Py_DECREF(outcode); | ||
|
|
||
| descr = PyArray_DescrFromScalar(self); |
There was a problem hiding this comment.
This line is repeated now, maybe we can merge them
There was a problem hiding this comment.
I'm less comfortable in python c code, and I consolidated two Py_DECREF to one place which felt better but made me question why they were seperated before. Can you give my update a through inspection.
There was a problem hiding this comment.
You missed that descr is still being used further down in new = ...
There was a problem hiding this comment.
descr is a better name than outcode anyway
There was a problem hiding this comment.
You missed that
descris still being used further down innew = ...
I'm not sure what you're implying by this? I dropped outcode so I think desc should still be used in the new = ....
There was a problem hiding this comment.
Yes, but as long as you are using descr you must hold a reference to it. If you call DECREF you lose that reference (i.e. if you wanted to be super clean, at least in this case, you might even do descr = NULL or similar incarnations to point out that it is not anymore valid to use).
There was a problem hiding this comment.
Thanks, I was being silly and only making sure the count was right at the end of the function. Fixed now.
bdadea3 to
33d8176
Compare
33d8176 to
7e8c0d6
Compare
This was proposed in one of the C cleanups, I tried it out and it looked better.