[Python-Dev] builtin filter function
Hello.
I was reading source code for bltinmodule.c and found probably erroneus
stuff in filter function. I'm newbie to python inners and don't know if
attached code is worth for a patch submission.
I would appreciate if someone could take a look at it and if it's ok
then i'll make submission, otherwise just drop it.
TIA
Ruslan
Index: bltinmodule.c
===
RCS file: /cvsroot/python/python/dist/src/Python/bltinmodule.c,v
retrieving revision 2.321
diff -c -r2.321 bltinmodule.c
*** bltinmodule.c 11 Mar 2005 06:49:40 - 2.321
--- bltinmodule.c 19 Jul 2005 21:02:30 -
***
*** 224,231
/* Pre-allocate argument list tuple. */
arg = PyTuple_New(1);
! if (arg == NULL)
! goto Fail_arg;
/* Get a result list. */
if (PyList_Check(seq) && seq->ob_refcnt == 1) {
--- 224,233
/* Pre-allocate argument list tuple. */
arg = PyTuple_New(1);
! if (arg == NULL) {
! Py_DECREF(it);
! return NULL;
! }
/* Get a result list. */
if (PyList_Check(seq) && seq->ob_refcnt == 1) {
***
*** 295,301
Py_DECREF(result);
Fail_it:
Py_DECREF(it);
- Fail_arg:
Py_DECREF(arg);
return NULL;
}
--- 297,302
***
*** 525,531
return NULL;
}
if (globals != Py_None && !PyDict_Check(globals)) {
! PyErr_SetString(PyExc_TypeError, PyMapping_Check(globals) ?
"globals must be a real dict; try eval(expr, {}, mapping)"
: "globals must be a dict");
return NULL;
--- 526,532
return NULL;
}
if (globals != Py_None && !PyDict_Check(globals)) {
! PyErr_SetString(PyExc_TypeError, PyMapping_Check(globals) ?
"globals must be a real dict; try eval(expr, {}, mapping)"
: "globals must be a dict");
return NULL;
***
*** 1190,1200
if (kwds != NULL && PyDict_Check(kwds) && PyDict_Size(kwds)) {
keyfunc = PyDict_GetItemString(kwds, "key");
if (PyDict_Size(kwds)!=1 || keyfunc == NULL) {
! PyErr_Format(PyExc_TypeError,
"%s() got an unexpected keyword argument", name);
return NULL;
}
! }
it = PyObject_GetIter(v);
if (it == NULL)
--- 1191,1201
if (kwds != NULL && PyDict_Check(kwds) && PyDict_Size(kwds)) {
keyfunc = PyDict_GetItemString(kwds, "key");
if (PyDict_Size(kwds)!=1 || keyfunc == NULL) {
! PyErr_Format(PyExc_TypeError,
"%s() got an unexpected keyword argument", name);
return NULL;
}
! }
it = PyObject_GetIter(v);
if (it == NULL)
***
*** 1908,1914
Py_DECREF(newlist);
return NULL;
}
!
newargs = PyTuple_GetSlice(args, 1, 4);
if (newargs == NULL) {
Py_DECREF(newlist);
--- 1909,1915
Py_DECREF(newlist);
return NULL;
}
!
newargs = PyTuple_GetSlice(args, 1, 4);
if (newargs == NULL) {
Py_DECREF(newlist);
***
*** 2536,2556
if (ok) {
int reslen;
if (!PyUnicode_Check(item)) {
! PyErr_SetString(PyExc_TypeError,
"can't filter unicode to unicode:"
" __getitem__ returned different type");
Py_DECREF(item);
goto Fail_1;
}
reslen = PyUnicode_GET_SIZE(item);
! if (reslen == 1)
PyUnicode_AS_UNICODE(result)[j++] =
PyUnicode_AS_UNICODE(item)[0];
else {
/* do we need more space? */
int need = j + reslen + len - i - 1;
if (need > outlen) {
! /* overallocate,
to avoid reallocations */
if (need < 2 * outlen)
need = 2 * outlen;
--- 2537,2557
if (ok) {
int reslen;
if (!PyUnicode_Check(item)) {
! PyErr_SetString(PyExc_TypeError,
"can't filter unicode to unicode:"
" __getitem__ returned different type");
Py_DECREF(item);
goto Fail_1;
}
reslen = PyUnicode_GET_SIZE(item);
! if (reslen == 1)
PyUnicode_AS_UNICODE(result)[j++] =
PyUnicode_AS_UNICODE(item)[0];
else {
/* do we need more space? */
int need = j + reslen + len - i - 1;
if (need > outlen) {
! /* overallocate,
to avoid reallocations */
if (need < 2 * outlen)
need = 2 * outlen;
___
Python-Dev mailing list
Python-Dev@python.org
http://mail.python.org/mailman/listinfo/python-dev
Unsubscribe:
http://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
Re: [Python-Dev] builtin filter function
Ruslan Spivak wrote: > Hello. > > I was reading source code for bltinmodule.c and found probably erroneus > stuff in filter function. I'm newbie to python inners and don't know if > attached code is worth for a patch submission. > > I would appreciate if someone could take a look at it and if it's ok > then i'll make submission, otherwise just drop it. > TIA This is indeed a bug in the function, and could have caused memory leaks because of not releasing the preallocated tuple. Since this is evident, I fixed it by now (Python/bltinmodule.c r2.318.2.1 and r2.322), but in the future you should always post patches to SourceForge to avoid crowding python-dev. But above all, thank you for spotting this issue and helping to make Python more bug-free! Reinhold -- Mail address is perfectly valid! ___ Python-Dev mailing list Python-Dev@python.org http://mail.python.org/mailman/listinfo/python-dev Unsubscribe: http://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
Re: [Python-Dev] builtin filter function
В Срд, 20/07/2005 в 00:27 +0200, Reinhold Birkenfeld пишет: > Ruslan Spivak wrote: > > Hello. > > > > I was reading source code for bltinmodule.c and found probably erroneus > > stuff in filter function. I'm newbie to python inners and don't know if > > attached code is worth for a patch submission. > > > > I would appreciate if someone could take a look at it and if it's ok > > then i'll make submission, otherwise just drop it. > > TIA > > This is indeed a bug in the function, and could have caused memory leaks > because of not releasing the preallocated tuple. > > Since this is evident, I fixed it by now (Python/bltinmodule.c r2.318.2.1 and > r2.322), but in the future you should always post patches to SourceForge to > avoid crowding python-dev. Thanks, i promise next time i'll post to SourceForge :) > > But above all, thank you for spotting this issue and helping to make Python > more bug-free! Glad i was somehow helpful. Ruslan ___ Python-Dev mailing list Python-Dev@python.org http://mail.python.org/mailman/listinfo/python-dev Unsubscribe: http://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
Re: [Python-Dev] builtin filter function
Reinhold Birkenfeld wrote: > Ruslan Spivak wrote: >> Hello. >> >> I was reading source code for bltinmodule.c and found probably erroneus >> stuff in filter function. I'm newbie to python inners and don't know if >> attached code is worth for a patch submission. >> >> I would appreciate if someone could take a look at it and if it's ok >> then i'll make submission, otherwise just drop it. >> TIA > > This is indeed a bug in the function, and could have caused memory leaks > because of not releasing the preallocated tuple. Correction: should have been "not releasing the iterator". (However, the bug would have been triggered only if the tuple could not be allocated, which is a very unlikely case). Reinhols -- Mail address is perfectly valid! ___ Python-Dev mailing list Python-Dev@python.org http://mail.python.org/mailman/listinfo/python-dev Unsubscribe: http://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
Re: [Python-Dev] builtin filter function
On 7/19/05, Ruslan Spivak <[EMAIL PROTECTED]> wrote: > I was reading source code for bltinmodule.c and found probably erroneus > stuff in filter function. I'm newbie to python inners and don't know if > attached code is worth for a patch submission. > > I would appreciate if someone could take a look at it and if it's ok > then i'll make submission, otherwise just drop it. As was already said, thanks for your contribution! I noticed in your patch that you also did "whitespace normalization" of the file before diffing it. This is generally not a good idea -- it distracts the reader of the patch from the actual bug the patch is fixing. In your case, 4 out of 6 patch chunks were spurious. We do like to keep our whitespace normalized, but as a rule we only do this in patches that don't make otherwise significant changes, so that the semantic changes are separate from the cleanups. That's a minor nit, however -- thanks for finding the memory leak! -- --Guido van Rossum (home page: http://www.python.org/~guido/) ___ Python-Dev mailing list Python-Dev@python.org http://mail.python.org/mailman/listinfo/python-dev Unsubscribe: http://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
Re: [Python-Dev] builtin filter function
В Втр, 19/07/2005 в 17:45 -0700, Guido van Rossum пишет: > On 7/19/05, Ruslan Spivak <[EMAIL PROTECTED]> wrote: > > I was reading source code for bltinmodule.c and found probably erroneus > > stuff in filter function. I'm newbie to python inners and don't know if > > attached code is worth for a patch submission. > > > > I would appreciate if someone could take a look at it and if it's ok > > then i'll make submission, otherwise just drop it. > > As was already said, thanks for your contribution! > > I noticed in your patch that you also did "whitespace normalization" > of the file before diffing it. This is generally not a good idea -- it > distracts the reader of the patch from the actual bug the patch is > fixing. In your case, 4 out of 6 patch chunks were spurious. > > We do like to keep our whitespace normalized, but as a rule we only do > this in patches that don't make otherwise significant changes, so that > the semantic changes are separate from the cleanups. > Thanks for this note, i'll keep that in mind next time. Ruslan ___ Python-Dev mailing list Python-Dev@python.org http://mail.python.org/mailman/listinfo/python-dev Unsubscribe: http://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
