Martin v. Löwis mar...@v.loewis.de added the comment:
I have now committed this patch with the proposed modifications as r73014.
I don't believe that the additional destr call is necessary, as
releasing the capsule will invoke destr already.
--
resolution: - fixed
status: open -
Benjamin Peterson benja...@python.org added the comment:
Patch review:
- It needs docs and tests.
- In addcleanup_convert, Py_FatalError calls abort, so there isn't
really a point of returning -1;
- Also in the function, you need to call destr() in the case that
PyList_Append fails.
--
Martin v. Löwis mar...@v.loewis.de added the comment:
I would propose a different approach: the same function can be both
argument parser and cleanup function. In parsing, the PyObject* points
to the parameter being converted. In cleanup, it points to NULL.
This would allow the O to continue to
Martin v. Löwis mar...@v.loewis.de added the comment:
Well, this is not implemented yet. (I introduced another format
temporary) Does this mean converter should return Py_CLEANUP_SUPPORTED
instead of 1 when cleanup is needed?
If you are introducing a new character ($), then this isn't
Martin v. Löwis mar...@v.loewis.de added the comment:
Modifying convert_to_unicode is incorrect; this function is not an O
converter. Instead, PyUnicode_FSConverter needs to change.
Attached is a patch that does that, and also uses the O approach. It
also adjusts the patch to use capsules.
Martin v. Löwis mar...@v.loewis.de added the comment:
So: any opinions what approach would be better? A new converter O$ or a
change to the existing O, selected by return value from the conversion
function?
--
___
Python tracker
Antoine Pitrou pit...@free.fr added the comment:
Reusing O looks better to me (we already have too many type specifiers).
Why have you chosen 0x2 for Py_CLEANUP_SUPPORTED? Are there other
possible values below that?
--
nosy: +pitrou
___
Python
Martin v. Löwis mar...@v.loewis.de added the comment:
Reusing O looks better to me (we already have too many type specifiers).
Why have you chosen 0x2 for Py_CLEANUP_SUPPORTED? Are there other
possible values below that?
It's essentially random. I want it to be a power of two, in case
Hirokazu Yamamoto ocean-c...@m2.ccsnet.ne.jp added the comment:
Reusing O looks better to me
Me too.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue6012
___
Hirokazu Yamamoto ocean-c...@m2.ccsnet.ne.jp added the comment:
Modifying convert_to_unicode is incorrect; this function is not an O
converter. Instead, PyUnicode_FSConverter needs to change.
Well, convert_to_unicode used to use O before, and I fixed memory leak
with O formatter in current
Hirokazu Yamamoto ocean-c...@m2.ccsnet.ne.jp added the comment:
Well, please see r65745. That was O before.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue6012
___
Hirokazu Yamamoto ocean-c...@m2.ccsnet.ne.jp added the comment:
the same function can be both
argument parser and cleanup function
Here is an another experimental patch to implement this. (Is this right
usage of PyCObject_FromVoidPtrAndDesc?)
As not all converters would need or support
New submission from Hirokazu Yamamoto ocean-c...@m2.ccsnet.ne.jp:
This issue comes from #5990.
Currently, O doesn't accept general cleanup function. This causes
memory leak sometimes. Here is an experimental patch to create new
format O similar to O but have general cleanup function.
Changes by Antoine Pitrou pit...@free.fr:
--
nosy: +loewis
priority: - normal
stage: - patch review
type: - resource usage
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue6012
___
14 matches
Mail list logo
