Author skrah
Recipients josh.r, jtaylor, neologix, njs, pitrou, python-dev, skrah, vstinner
Date 2014-05-03.19:29:58
SpamBayes Score -1.0
Marked as misclassified Yes
Message-id <>
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:

        >>> 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

        >>> x = bytearray(0)
        >>> m = memoryview(x)
        >>> x.__init__(10)
        >>> x[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:59skrahsetrecipients: + skrah, pitrou, vstinner, njs, neologix, python-dev, jtaylor, josh.r
2014-05-03 19:29:59skrahsetmessageid: <>
2014-05-03 19:29:59skrahlinkissue21233 messages
2014-05-03 19:29:58skrahcreate