[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