Implement portable array pickling.

[Martin v. Löwis, 2009-07-06 04:54:53]

http://codereview.appspot.com/87072/diff/1/3
File Modules/arraymodule.c (left):

http://codereview.appspot.com/87072/diff/1/3#oldcode629
Line 629: array_copy(arrayobject *a, PyObject *unused)
Why did you remove these parameters? It's important that they stay, and their
removal seems unrelated to this patch, anyway.

http://codereview.appspot.com/87072/diff/1/3
File Modules/arraymodule.c (right):

http://codereview.appspot.com/87072/diff/1/3#newcode1059
Line 1059: 
bogus white-space only change.

http://codereview.appspot.com/87072/diff/1/3#newcode1504
Line 1504: UNKNOWN_FORMAT = -1,
Is it really necessary to support this UNKNOWN_FORMAT? I propose to drop it.

If it is necessary, a comment should explain the list of cases under which it
may get used.

http://codereview.appspot.com/87072/diff/1/3#newcode1654
Line 1654: static PyObject *
It would be good if a comment explained what this function is supposed to do.

http://codereview.appspot.com/87072/diff/1/3#newcode1664
Line 1664: typecode_obj = PyString_FromStringAndSize((char *)&typecode, 1);
That sounds like a typing error: you shouldn't reinterpret an int as a char[].

Are you sure this code actually works on big-endian machines, where &typecode
points to the MSB?

http://codereview.appspot.com/87072/diff/1/3#newcode1760
Line 1760: return NULL;
This leaks converted_items.

http://codereview.appspot.com/87072/diff/1/3#newcode1806
Line 1806: case UNSIGNED_INT8:
I think this integer conversion is incorrect (see msg71050). When unpickling an
l-code array on a 64-bit machine that was pickled on a 32-bit machine, I think
it should unpickle as an i-code array instead.

http://codereview.appspot.com/87072/diff/1/3#newcode1822
Line 1822: Py_ssize_t itemcount = Py_SIZE(items) / width;
This should check for truncation, IMO.

http://codereview.appspot.com/87072/diff/1/3#newcode1823
Line 1823: const unsigned char *memstr = \
The backslash isn't necessary here.

http://codereview.appspot.com/87072/diff/1/3#newcode1888
Line 1888: /* We got something wierd; so convert the array to a list
weird.

[Alexandre Vassalotti, 2009-07-06 23:12:31]

On 2009/07/06 04:54:53, Martin v. Löwis wrote:
> http://codereview.appspot.com/87072/diff/1/3#newcode1822
> Line 1822: Py_ssize_t itemcount = Py_SIZE(items) / width;
> This should check for truncation, IMO.
>

How can truncation occur? That's the only thing I didn't fix in my new patch
set.