Issue3668
Created on 2008-08-24 21:02 by amaury.forgeotdarc, last changed 2008-08-29 18:42 by pitrou.
|
msg71870 - (view) |
Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) |
Date: 2008-08-24 21:02 |
|
When PyArg_ParseTuple correctly parses a s* format, but raises an
exception afterwards (for a subsequent parameter), the user code will
not call PyBuffer_Release() and memory will leak.
Seen by "regrtest -R:: test_binascii"
For example:
>>> binascii.a2b_qp("", **{1:1})
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
TypeError: keywords must be strings
[42278 refs]
>>> binascii.a2b_qp("", **{1:1})
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
TypeError: keywords must be strings
[42279 refs]
>>> binascii.a2b_qp("", **{1:1})
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
TypeError: keywords must be strings
[42280 refs]
The same pattern was correctly handled by the "et#" type (where the user
has to call PyMem_Free) with the help of a cleanup list (see the
addcleanup() function in getargs.c). (See issue501716)
|
|
msg71917 - (view) |
Author: Antoine Pitrou (pitrou) |
Date: 2008-08-25 12:38 |
|
Here is a patch. Please review.
|
|
msg71918 - (view) |
Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) |
Date: 2008-08-25 12:57 |
|
This patch elegantly reuses the existing cleanup list.
Only two remarks:
- there are a few tabs/spaces inconsistencies.
- I would make the cleanup_ptr explicit:
intead of
addcleanup(*buffer, freelist, NULL);
I'd prefer
addcleanup(*buffer, freelist, cleanup_ptr);
|
|
msg71923 - (view) |
Author: Antoine Pitrou (pitrou) |
Date: 2008-08-25 13:42 |
|
Ok, here is a new patch addressing Amaury's comments.
|
|
msg71924 - (view) |
Author: Antoine Pitrou (pitrou) |
Date: 2008-08-25 14:02 |
|
By the way, this bug affects 2.6 as well, although not in binascii since
it has not been converted to use "s*":
>>> codecs.latin_1_decode(b"", 0)
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
TypeError: latin_1_decode() argument 2 must be string or None, not int
[57425 refs]
>>> codecs.latin_1_decode(b"", 0)
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
TypeError: latin_1_decode() argument 2 must be string or None, not int
[57426 refs]
|
|
msg71925 - (view) |
Author: Antoine Pitrou (pitrou) |
Date: 2008-08-25 14:19 |
|
Here is a patch for 2.6.
|
|
msg71926 - (view) |
Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) |
Date: 2008-08-25 14:27 |
|
Both patches look good to me.
|
|
msg71927 - (view) |
Author: Antoine Pitrou (pitrou) |
Date: 2008-08-25 14:31 |
|
Actually, here is a better patch for py3k.
|
|
msg71929 - (view) |
Author: Antoine Pitrou (pitrou) |
Date: 2008-08-25 14:39 |
|
And a similarly better patch for 2.6.
|
|
msg72123 - (view) |
Author: Antoine Pitrou (pitrou) |
Date: 2008-08-29 10:00 |
|
Amaury, are these patches ok to check in?
|
|
msg72126 - (view) |
Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) |
Date: 2008-08-29 11:50 |
|
Yes, let them go in!
|
|
msg72149 - (view) |
Author: Antoine Pitrou (pitrou) |
Date: 2008-08-29 18:42 |
|
Committed in r66057 and r66058.
|
|
| Date |
User |
Action |
Args |
| 2008-08-29 18:42:00 | pitrou | set | status: open -> closed resolution: accepted -> fixed messages:
+ msg72149 |
| 2008-08-29 11:50:52 | amaury.forgeotdarc | set | resolution: accepted messages:
+ msg72126 |
| 2008-08-29 10:00:06 | pitrou | set | messages:
+ msg72123 |
| 2008-08-25 14:39:57 | pitrou | set | files:
+ argleak2-2.6.patch messages:
+ msg71929 versions:
+ Python 2.6 |
| 2008-08-25 14:31:42 | pitrou | set | files:
+ argleak3.patch messages:
+ msg71927 |
| 2008-08-25 14:27:36 | amaury.forgeotdarc | set | messages:
+ msg71926 |
| 2008-08-25 14:19:37 | pitrou | set | files:
+ argleak-2.6.patch messages:
+ msg71925 |
| 2008-08-25 14:02:11 | pitrou | set | messages:
+ msg71924 |
| 2008-08-25 13:42:07 | pitrou | set | files:
+ argleak2.patch messages:
+ msg71923 |
| 2008-08-25 12:57:07 | amaury.forgeotdarc | set | messages:
+ msg71918 |
| 2008-08-25 12:38:59 | pitrou | set | keywords:
+ patch, needs review files:
+ argleak.patch type: resource usage messages:
+ msg71917 |
| 2008-08-25 10:07:17 | pitrou | set | assignee: pitrou |
| 2008-08-25 09:32:01 | pitrou | set | nosy:
+ pitrou |
| 2008-08-24 21:02:57 | amaury.forgeotdarc | create | |
|