Message217826
I did a post-commit review. A couple of things:
1) I think Victor and I have a different view of the calloc() parameters.
calloc(size_t nmemb, size_t size)
If a memory region of bytes is allocated, IMO 'nbytes' should be in the
place of 'nmemb' and '1' should be in the place of 'size'. That is,
"allocate nbytes elements of size 1":
calloc(nbytes, 1)
In the commit the parameters are reversed in many places, which confuses
me quite a bit, since it means "allocate one element of size nbytes".
calloc(1, nbytes)
2) I'm not happy with the refactoring in bytearray_init(). I think it would
be safer to make focused minimal changes in PyByteArray_Resize() instead.
In fact, there is a behavior change which isn't correct:
Before:
=======
>>> x = bytearray(0)
>>> m = memoryview(x)
>>> x.__init__(10)
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
BufferError: Existing exports of data: object cannot be re-sized
Now:
====
>>> x = bytearray(0)
>>> m = memoryview(x)
>>> x.__init__(10)
>>> x[0]
0
>>> m[0]
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
IndexError: index out of bounds
3) Somewhat similarly, I wonder if it was necessary to refactor
PyBytes_FromStringAndSize(). I find the new version more difficult
to understand.
4) _PyObject_Alloc(): assert(nelem <= PY_SSIZE_T_MAX / elsize) can be called
with elsize = 0. |
|
Date |
User |
Action |
Args |
2014-05-03 19:29:59 | skrah | set | recipients:
+ skrah, pitrou, vstinner, njs, neologix, python-dev, jtaylor, josh.r |
2014-05-03 19:29:59 | skrah | set | messageid: <1399145399.39.0.348175601691.issue21233@psf.upfronthosting.co.za> |
2014-05-03 19:29:59 | skrah | link | issue21233 messages |
2014-05-03 19:29:58 | skrah | create | |
|