> > On Mon, Apr 14, 2008 at 6:02 AM, Trent Nelson <[EMAIL PROTECTED]> 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. _______________________________________________ Python-3000 mailing list Python-3000@python.org http://mail.python.org/mailman/listinfo/python-3000 Unsubscribe: http://mail.python.org/mailman/options/python-3000/archive%40mail-archive.com