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
[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
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
