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
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!
В Срд, 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
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!
On 7/19/05, Ruslan Spivak rspivak@nuxeo.com 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/)
В Втр, 19/07/2005 в 17:45 -0700, Guido van Rossum пишет:
On 7/19/05, Ruslan Spivak rspivak@nuxeo.com 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
-
Guido van Rossum -
Reinhold Birkenfeld -
Ruslan Spivak