[Python-3000] longobject.c and Windows x64

[Trent Nelson]

> > On Mon, Apr 14, 2008 at 6:02 AM, Trent Nelson <tnelson at onresolve.com> wrote:
> >
> > On Windows x64, sizeof(size_t) > sizeof(long), so the existing
> > PyLong_FromSsize_t and PyLong_FromSize_t implementations in longobject.c
> > are just plain wrong.  I've patched it as follows, but as I'm not well
> > versed in the many intricacies of longobject.c, I'd appreciate input
> > from others.
>
> I'm missing something:  in what way are the existing implementations
> wrong?  I see that the test  (ival < PyLong_BASE) in
> PyLong_FromSsize_t should be something like:
> (ival < PyLong_BASE && ival > -PyLong_BASE),

Yeah, that's the 'wrong' part I was referring to.  I guess I wanted to bring that issue up as well as question the actual implementation.  For example, if we fixed the if statement, we'd having something looking like:

PyObject *
PyLong_FromSsize_t(Py_ssize_t ival)
{
       Py_ssize_t bytes = ival;
       int one = 1;
       if (ival < PyLong_BASE && ival > -PyLong_BASE)
               return PyLong_FromLong(ival);
       return _PyLong_FromByteArray(
                       (unsigned char *)&bytes,
                       SIZEOF_SIZE_T, IS_LITTLE_ENDIAN, 1);
}

I don't understand why we'd be interested in testing for the PyLong_FromLong() shortcut, then reverting to _PyLong_FromByteArray(), when we know we're always going to be dealing with a Py_ssize_t.  Speed?  Safety?  Cut and paste that went awry?  Why not just call the correct PyLong_FromLong(Long)() depending on sizeof(size_t) and be done with it?

> Couldn't PyLong_FromSsize_t be written to exactly mimic
> PyLong_FromLongLong?  It means duplication of code, I know,
> but it also means not relying on ssize_t being equal to either
> long or long long.

Surely, if we're guarding properly with #error in the case where sizeof(size_t) not in (sizeof(long), sizeof(Py_LONG_LONG)), reusing existing methods that do exactly what we want to do would be better than mimicking them?

> By the way, I don't much like the handling of negative
> values in PyLong_FromLong and PyLong_FromLongLong:
> these functions use code like:
>
>     if (ival < 0) {
>         ival = -ival;
>         negative = 1;
>     }
>
> which looks to me as though it might mishandle the case
> where ival = LONG_MIN.  Should this be fixed?

Ah, interesting.  Stepped through PyLong_FromLong via the following:

Python 3.0a4+ (py3k, Apr 14 2008, 18:44:17) [MSC v.1500 64 bit (AMD64)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> import _testcapi as t
>>> t.getargs_l(t.LONG_MIN)
-2147483648
>>> t.getargs_l(t.LONG_MIN+1)
-2147483647

When ival == LONG_MIN, the 'ival = -ival' statement doesn't have any effect on the value of ival, it stays as LONG_MIN.  (With LONG_MIN+1 though, ival does correctly get cast back into the positive realm...)  This isn't causing a problem (at least not on Windows) as ival's cast to an unsigned long later in the method -- I wonder if all platforms/compilers silently ignore 'ival = -ival' when at LONG_MIN though...


        Trent.