[issue6012] enhance getargs O& to accept cleanup function

[Martin v . Löwis]

Martin v. Löwis  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 -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue6012] enhance getargs O& to accept cleanup function

[Benjamin Peterson]

Benjamin Peterson  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.

--
nosy: +benjamin.peterson

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue6012] enhance getargs O& to accept cleanup function

[Martin v. Löwis]

Martin v. Löwis  added the comment:

> Well, convert_to_unicode used to use O& before

I don't understand. Where does it use it? Before
your patch, convert_to_unicode was a regular one-argument
function, not a ParseTuple O& function. Only your patch
makes it one.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue6012] enhance getargs O& to accept cleanup function

[Martin v. Löwis]

Martin v. Löwis  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.

--
Added file: http://bugs.python.org/file13997/conv.diff

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue6012] enhance getargs O& to accept cleanup function

[Martin v. Löwis]

Changes by Martin v. Löwis :


--
priority: normal -> release blocker

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue6012] enhance getargs O& to accept cleanup function

[Martin v. Löwis]

Martin v. Löwis  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 actually
needed. I was thinking of reusing O&, literally.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue6012] enhance getargs O& to accept cleanup function

[Martin v. Löwis]

Martin v. Löwis  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 be used even if cleanup is
needed. As not all converters would need or support cleanup, converters
that do need cleanup could be required to return Py_CLEANUP_SUPPORTED
(which would be, say, 131072).

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue6012] enhance getargs O& to accept cleanup function

[Hirokazu Yamamoto]

Hirokazu Yamamoto  added the comment:

> Reusing O& looks better to me

Me too.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue6012] enhance getargs O& to accept cleanup function

[Martin v . Löwis]

Martin v. Löwis  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 we make
it a bit mask some day, and significantly different from 1. 0x2 was
the smallest number that qualified :-)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue6012] enhance getargs O& to accept cleanup function

[Antoine Pitrou]

Antoine Pitrou  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 tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue6012] enhance getargs O& to accept cleanup function

[Martin v . Löwis]

Martin v. Löwis  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 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue6012] enhance getargs O& to accept cleanup function

[Hirokazu Yamamoto]

Hirokazu Yamamoto  added the comment:

Well, please see r65745. That was O& before.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue6012] enhance getargs O& to accept cleanup function

[Hirokazu Yamamoto]

Hirokazu Yamamoto  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 way. I used this function because I know
about it more than PyUnicode_FSConverter. Sorry for confusion. ;-)

> Attached is a patch that does that, and also uses the O& approach. It
> also adjusts the patch to use capsules.

Oh, this is how to use new capsule functions I noticed capsule.c was
added, but I didn't know how to use it.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue6012] enhance getargs O& to accept cleanup function

[Hirokazu Yamamoto]

Hirokazu Yamamoto  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 cleanup, converters
> that do need cleanup could be required to return Py_CLEANUP_SUPPORTED

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?

--
Added file: 
http://bugs.python.org/file13982/experimental_getargs_with_cleanup_v2.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue6012] enhance getargs O& to accept cleanup function

[Antoine Pitrou]

Changes by Antoine Pitrou :


--
nosy: +loewis
priority:  -> normal
stage:  -> patch review
type:  -> resource usage

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue6012] enhance getargs O& to accept cleanup function

[Hirokazu Yamamoto]

New submission from Hirokazu Yamamoto :

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.

--
components: Interpreter Core
files: experimental_getargs_with_cleanup.patch
keywords: patch
messages: 87677
nosy: ocean-city
severity: normal
status: open
title: enhance getargs O& to accept cleanup function
versions: Python 2.7, Python 3.1
Added file: 
http://bugs.python.org/file13976/experimental_getargs_with_cleanup.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com