Repository navigation
"s*" argument parser marker leaks memory #47918
Description
Activity
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 bpo-501716)Here is a patch. Please review.
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);
Ok, here is a new patch addressing Amaury's comments.
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]
Here is a patch for 2.6.
Both patches look good to me.
Actually, here is a better patch for py3k.
And a similarly better patch for 2.6.
Amaury, are these patches ok to check in?
Yes, let them go in!
Committed in r66057 and r66058.
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields: