[Python-Dev] r84430 - in python/branches/py3k: Include/unicodeobject.h Objects/unicodeobject.c

[Victor Stinner]

Hi,

Le jeudi 02 septembre 2010 11:13:22, vous avez écrit :
> > 1. This function and PyUnicode_strcat are missing documentation.

It's Py_UNICODE_strcat(), not PyUnicode_strcat(). But yes, Py_UNICODE_strcat() 
is not documented. But I didn't found any doc for other Py_UNICODE_str*() 
functions in Doc/c-api/*.rst. Does it mean that we should write a document for 
all these functions? They mimic the str*() C API.

If PyUnicode_strdup() is kept public, it should be documented, yes.

--

> If you want to make this a public API function, it also needs to be
> renamed to fit the rest of the API.

PyUnicode_strdup(): I don't like this name. In my first patch, it was called 
Py_UNICODE_strdup() but all Py_UNICODE_*() functions take Py_UNICODE* 
arguments, no PyUnicodeObject* argument.

> I'd suggest PyUnicode_AsUnicodeCopy() which then corresponds
> to PyUnicode_FromUnicode(), but I'm not sure whether we should
> have such a public API in the first place.

PyUnicode_AsUnicodeCopy() is maybe a better name, I like it.

--

> What are they going to be used for?

I am not able to merge my huge work on unicode import (see #8611, #9425 and 
the "import_unicode" svn branch) at once, so I'm splitting it into atomic 
commits. PyUnicode_strdup() and Py_UNICODE_strcat() are not used yet, but I 
plan to use them for the import machinery (Python/import.c) to replace char* 
by Py_UNICODE*.

> > 2. Are you sure they need to be public APIs?

About Py_UNICODE_strcat(): yes, why not? Other Py_UNICODE_str*() functions are 
pratical but also required to manipulate Py_UNICODE* strings.

About PyUnicode_strdup() (PyUnicode_AsUnicodeCopy): I don't know. It is 
possible to rewrite it in few lines. Why don't you want to add them to the 
public API? For my work, it doesn't matter if it's public or not. This 
function uses PyMem_xxx API, I don't know if a third part library would like 
to rely on PyMem_xxx.

-- 
Victor Stinner
http://www.haypocalc.com/