| 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 "", line 1, in TypeError: keywords must be strings [42278 refs] >>> binascii.a2b_qp("", **{1:1}) Traceback (most recent call last): File "", line 1, in TypeError: keywords must be strings [42279 refs] >>> binascii.a2b_qp("", **{1:1}) Traceback (most recent call last): File "", line 1, in 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 ) |
|
|
| 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 "", line 1, in 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 "", line 1, in 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. |
|
|