Message49574
Logged In: YES
user_id=33168
I see one problem in Modules/operator.c. This code:
+ if (i==-1 && PyErr_Occurred()) return NULL;
+ else return PyInt_FromLong((long)i);
Should be:
+ if (i == -1 && PyErr_Occurred())
+ return NULL;
+ return PyInt_FromSsizeT(i);
There was a discussion on python-checkins recently about
this. The short answer is that if Py_ssize_t's are cast to
a long, that will lose precision on Win64 (size_ts are
64-bits, but longs are still 32). Since i is Py_ssize_t,
the cast would lose the high order values (above 2G).
In Objects/stringobject.c:
- i += PyString_GET_SIZE(self);
- return string_item(self,i);
+ i += PyList_GET_SIZE(self);
+ return string_item(self, i);
I don't see how that (PyList) can be correct, self is a
string object.
I would change this code:
+ lenfunc func;
+ PyNumberMethods *nb;
+ if ((nb=item->ob_type->tp_as_number) && \
+ (func=nb->nb_index)) {
+ Py_ssize_t i = func(item);
to:
+ PyNumberMethods *nb = item->ob_type->tp_as_number;
+ if (nb != NULL && nb->nb_index != NULL) { + Py_ssize_t i
= nb->nb_index(item);
Let the compiler do the work. We can optimize later if this
is a problem. I think the code is clearer this way and is
more similar to the rest of the code. I don't care as much
about the use of func here as the change to nb = ... If you
want to keep the local variable, I would prefer a better
name like nb_index instead of func.
There's no need for a \ after &&.
Most of these comments apply to Objects/listobject.c,
Objects/tupleobject.c, Objects/unicodeobject.c, PyNumber_Index.
In Objects/classobject.c, you don't check that indexstr is
non-NULL after assignment. In instance_index(), remove the
cast (int) of the return result outcome.
In slot_nb_index() remove . from end of exception string.
Note: you don't need the Py_DECREF() and return -1 in the
last if as it would be the same if it fell through. Use
whichever version you think is clearer. Either way is fine
with me.
In Include/abstract.h, do all the comments still say a C
integer is returned even though a Py_ssize_t is returned?
If not, can you update your comment too. |
|
Date |
User |
Action |
Args |
2007-08-23 15:45:58 | admin | link | issue1436368 messages |
2007-08-23 15:45:58 | admin | create | |
|