On May 27, 2013 11:54 PM, "ZyX" <[email protected]> wrote: > > > 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. > > No longer the case after switching to StringToChars and using dictkey_todecref as a name. > > > 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. > > It is not the only one e.g. Py_BEGIN_ALLOW_THREADS does the same. Hiding common `bytes` declaration is no good, but specific `dictkey_todecref` is OK. > > > 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. > > Was fixed in some later patches. > > > 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. > > It does not matter much what to write, DICTKEY_UNREF or Py_(X)DECREF. What matters is that in some places you cannot just return err; you need to decref other objects as well. > > > 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. > > Nope, check out documentation. If it was so simple I would have used a function: due to > > > The buffer refers to an internal string buffer of obj, not a copy. > > I *must* keep a reference until I no longer need key. Or save a copy instead which is bad for performance. (I am using function accepting return-argument PyObject ** now though: it is set to an objects that is to be Py_XDECREFed.) > > > 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) > > Please refer to the latest patch in sequence. There *is* now a function. > > > . 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 > > This one is true, I usually avoid touching built-in functions.
Not actually true, StringToLine must handle zero byte in string and should forbid newline; StrinToChars must forbid zero byte and work fine with newline. > -- > -- > 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. > > -- -- 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.
