On Sat, May 25, 2013 at 10:47 AM, ZyX wrote:
> # HG changeset patch
> # User ZyX <[email protected]>
> # Date 1369465902 -14400
> # Node ID f3a1f7c373ee941b71739260f72f6babbff6ce0f
> # Parent e3fac877d315709055ffaec4cc32a5f971ef5e97
> Fix invalid read errors:
>
> defer DICTKEY_UNREF until key is no longer needed
>
The patch is not correct:
'bytes' is declared in OptionsAssItem(), in the block (line 1689):
'else if (PyUnicode_Check(valObject))'
This declaration shadows the declaration of 'bytes' made by DICTKEY_DECL at
the start of the function. Hence the shadowed 'bytes' PyObject is never
decreferenced whenever OptionsAssItem() returns from this block.
The DICTKEY_ family of macros have been introduced by patch v7-3-569. It seems
that all these macros should be removed.
DICTKEY_DECL:
Clearly a dangerous macro when the author of the macro himself can be fooled by
the fact that the macro hides a declaration.
DICTKEY_UNREF:
The macro should not test for 'bytes != NULL' as Py_XDECREF's purpose is
specifically to do this test before doing the decreferencing.
This macro hides a decreferencing and hides the object that is being
decreferenced. See
http://docs.python.org/2/extending/extending.html#reference-counting-in-python
for how critical it is to get the reference counting correct when
using the python/C API, to avoid crashes and memory leaks.
The 'bytes' object must be decreferenced as soon as not needed anymore, which
means inside the DICTKEY_GET macro and after the call to
PyString_AsStringAndSize(). So the macro is actually never needed.
DICTKEY_GET:
This is a macro with parameters, but missing most parameters (keyObject, key,
bytes) and therefore very obscure.
Its name is misleading, the purpose of the macro is to set a pointer to an
internal null terminated string that represents a string, a Unicode or a bytes
python object and there is no reference to a dictionary or of one of its keys.
Getting such a pointer is obviously often needed to interface vim with python
and this means that it should be a function. And indeed there are many other
parts in the code where this is implemented (however not consistently) and that
would benefit from using this function, for example:
. no need to look very far, right in the 'else if
(PyUnicode_Check(valObject))' block mentioned above, but here the
implementation forgets to use CODEC_ERROR_HANDLER (this is a bug)
. at the start of StringToLine() but here there is no error handling (this is
a bug) and the weird handling of 'bytes' in the case of Python 2 could be
avoided by using this function
--
Xavier
Les Chemins de Lokoti: http://lokoti.alwaysdata.net
--
--
You received this message from the "vim_dev" maillist.
Do not top-post! Type your reply below the text you are replying to.
For more information, visit http://www.vim.org/maillist.php
---
You received this message because you are subscribed to the Google Groups
"vim_dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.
