[PATCH] Fix python_access on 64bits hosts
Julien ÉLIE
julien at trigofacile.com
Sun Oct 31 11:13:11 UTC 2010
Bonjour Raphaël,
> I have just completed a standard (gentoo) installation of INN 2.5.1 on a
> 64bits host, and it seems that there is an issue with the "python_access"
> method.
[...]
> This seems to come from the use of an "int" for the iterator, whereas the
> Python doc uses a "Py_ssize_t" : http://docs.python.org/c-api/dict.html,
> "PyDict_Next" section.
Gosh! I read in the page you mention:
Changed in version 2.5: This function used an int * type for ppos.
This might require changes in your code for properly supporting 64-bit
systems.
That's a very nice catch, thanks!
Changes in Python 2.5:
PEP 353: Using ssize_t as the index type
A wide-ranging change to Python's C API, using a new Py_ssize_t type
definition instead of int, will permit the interpreter to handle more
data on 64-bit platforms. This change doesn't affect Python's capacity
on 32-bit platforms.
The change will cause incompatibilities on 64-bit machines, so it was
deemed worth making the transition now, while the number of 64-bit users
is still relatively small. (In 5 or 10 years, we may all be on 64-bit
machines, and the transition would be more painful then.)
Amusing rationale for the moment of the transition...
More information about this PEP (Python Enhancement Proposal):
http://www.python.org/dev/peps/pep-0353/
as each pointer is 8B (in a 64-bit machine), one needs 16GiB to just
hold the pointers of such a lis
Python 2.5 (released in September 2006) introduces a new Py_ssize_t type
and a few functions we are currently using, expect it on 64-bit platforms.
I do not think the way we are using Python in INN requires 64-bit integers!
We only compute:
* message-ID lengths
* article body lengths
* newsgroups status lengths in the active file
* syslog level char lengths
* string lengths in order to make a hash of them
No way they are that huge!
I therefore believe the most straight-forward fix is to use Py_ssize_t
for compatibility issues with current 64-bit Python versions:
/* Make "s#" use Py_ssize_t rather than int. */
#define PY_SSIZE_T_CLEAN
/* Define Py_ssize_t when it does not exist. */
#if PY_VERSION_HEX < 0x02050000
typedef int Py_ssize_t;
#endif
Note that your patch needs adding "typedef int Py_ssize_t" because otherwise
it breaks the build on Python version < 2.5.0.
PEP 353 provides a checker:
http://svn.effbot.python-hosting.com/stuff/sandbox/python/ssizecheck.py
innd/python.c:116: 'PyBuffer_FromMemory' uses Py_ssize_t for input parameters
innd/python.c:125: 'PyBuffer_FromMemory' uses Py_ssize_t for input parameters
innd/python.c:333: 'PyString_FromStringAndSize' uses Py_ssize_t for input parameters
innd/python.c:348: 'PyString_FromStringAndSize' uses Py_ssize_t for input parameters
innd/python.c:378: 'PyString_FromStringAndSize' uses Py_ssize_t for input parameters
innd/python.c:409: 'PyString_FromStringAndSize' uses Py_ssize_t for input parameters
innd/python.c:527: 'PyString_FromStringAndSize' uses Py_ssize_t for input parameters
nnrpd/python.c:125: 'PyBuffer_FromMemory' uses Py_ssize_t for input parameters
nnrpd/python.c:129: 'PyBuffer_FromMemory' uses Py_ssize_t for input parameters
nnrpd/python.c:137: 'PyBuffer_FromMemory' uses Py_ssize_t for input parameters
nnrpd/python.c:141: 'PyBuffer_FromMemory' uses Py_ssize_t for input parameters
nnrpd/python.c:152: 'PyBuffer_FromMemory' uses Py_ssize_t for input parameters
nnrpd/python.c:160: 'PyBuffer_FromMemory' uses Py_ssize_t for input parameters
nnrpd/python.c:171: 'PyTuple_Size' uses Py_ssize_t for return values (may need overflow check)
nnrpd/python.c:171: 'PyTuple_Size' uses Py_ssize_t for return values (may need overflow check)
nnrpd/python.c:179: 'PyTuple_GetItem' uses Py_ssize_t for input parameters
nnrpd/python.c:193: 'PyTuple_GetItem' uses Py_ssize_t for input parameters
nnrpd/python.c:207: 'PyTuple_Size' uses Py_ssize_t for return values (may need overflow check)
nnrpd/python.c:210: 'PyTuple_GetItem' uses Py_ssize_t for input parameters
nnrpd/python.c:268: 'PyBuffer_FromMemory' uses Py_ssize_t for input parameters
nnrpd/python.c:272: 'PyBuffer_FromMemory' uses Py_ssize_t for input parameters
nnrpd/python.c:280: 'PyBuffer_FromMemory' uses Py_ssize_t for input parameters
nnrpd/python.c:284: 'PyBuffer_FromMemory' uses Py_ssize_t for input parameters
nnrpd/python.c:292: 'PyBuffer_FromMemory' uses Py_ssize_t for input parameters
nnrpd/python.c:312: 'PyDict_Size' uses Py_ssize_t for return values (may need overflow check)
nnrpd/python.c:318: WARNING: 'PyDict_Next' uses Py_ssize_t for output parameters (must fix!)
nnrpd/python.c:395: 'PyBuffer_FromMemory' uses Py_ssize_t for input parameters
nnrpd/python.c:399: 'PyBuffer_FromMemory' uses Py_ssize_t for input parameters
nnrpd/python.c:407: 'PyBuffer_FromMemory' uses Py_ssize_t for input parameters
nnrpd/python.c:411: 'PyBuffer_FromMemory' uses Py_ssize_t for input parameters
nnrpd/python.c:419: 'PyBuffer_FromMemory' uses Py_ssize_t for input parameters
nnrpd/python.c:428: 'PyBuffer_FromMemory' uses Py_ssize_t for input parameters
nnrpd/python.c:432: 'PyBuffer_FromMemory' uses Py_ssize_t for input parameters
You pointed out the "must fix" :-)
The other warnings seem OK to me. Casts from int (used by strlen) to Py_ssize_t
are normally transparent.
Only calls to PyArg_ParseTuple() should be changed according to me, so as to
prepare our code for the day when PY_SSIZE_T_CLEAN is assumed by Python.
Patch included.
> I have attached a patch fixing this issue ; it compiles and runs fine on my
> host.
Ok, thanks.
We will one day also have to support Python 3... It currently does not compile
with our source code.
> Please let me know if you need more information / additional tests / ...
Could you please use the patch attached to this mail , compile INN with
"make warnings" and tell us whether other errors appear?
(even if not related to Python)
Thanks beforehand!
I am for instance unsure about:
/* Resize vector to dictionary length. */
vector_resize(access_vec, PyDict_Size(result) - 1);
Maybe (size_t) PyDict_Size(result) - 1 is needed. Please tell us if
"make warnings" complains.
I also wonder whether your new "pos" variable should not be casted to an int
during the following call:
syslog(L_ERROR, "python access method return dictionary key %i not a string",
pos);
Please also tell us if "make warnings" mentions something about that.
Have a nice week,
--
Julien ÉLIE
« -- Ce serait une joyeuse idée de faire une tempête dans nos cerveaux, n'est-il pas ?
-- Il veut parler d'une réunion de travail ! » (Astérix)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: patch-Py_ssize_t.diff
Type: application/octet-stream
Size: 4692 bytes
Desc: not available
URL: <https://lists.isc.org/pipermail/inn-workers/attachments/20101031/cca491d3/attachment.obj>
More information about the inn-workers
mailing list