Re: [gentoo-portage-dev] [PATCH 3/3] pym/portage/util/locale.py: add a C module to help check locale

[Michał Górny]

On Fri, 27 May 2016 10:26:44 -0400
"Anthony G. Basile" <basile@××××××××××××××.edu> wrote:

> From: "Anthony G. Basile" <blueness@g.o>
>
> The current method to check for a sane system locale is to use python's
> ctypes.util.find_library() to construct a full library path to the
> system libc.so and pass that path to ctypes.CDLL() so we can call
> toupper() and tolower() directly.  However, this gets bogged down in
> implementation details and fails with musl.
>
> We work around this design flaw in ctypes with a small python module
> written in C which provides thin wrappers to toupper() and tolower(),
> and only fall back on the current ctypes-based check when this module
> is not available.
>
> This has been tested on glibc, uClibc and musl systems.
>
> X-Gentoo-bug: 571444
> X-Gentoo-bug-url: https://bugs.gentoo.org/show_bug.cgi?id=571444
>
> Signed-off-by: Anthony G. Basile <blueness@g.o>
> ---
>  pym/portage/util/locale.py | 16 ++++++-----
>  setup.py                   |  6 +++-
>  src/portage_util_libc.c    | 70 ++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 84 insertions(+), 8 deletions(-)
>  create mode 100644 src/portage_util_libc.c
>
> diff --git a/pym/portage/util/locale.py b/pym/portage/util/locale.py
> index 093eb86..5b09945 100644
> --- a/pym/portage/util/locale.py
> +++ b/pym/portage/util/locale.py
> @@ -34,13 +34,15 @@ def _check_locale(silent):
>  	"""
>  	The inner locale check function.
>  	"""
> -
> -	libc_fn = find_library("c")
> -	if libc_fn is None:
> -		return None
> -	libc = LoadLibrary(libc_fn)
> -	if libc is None:
> -		return None
> +	try:
> +		from portage.util import libc
> +	except ImportError:
> +		libc_fn = find_library("c")
> +		if libc_fn is None:
> +			return None
> +		libc = LoadLibrary(libc_fn)
> +		if libc is None:
> +			return None
>
>  	lc = list(range(ord('a'), ord('z')+1))
>  	uc = list(range(ord('A'), ord('Z')+1))
> diff --git a/setup.py b/setup.py
> index 25429bc..5ca8156 100755
> --- a/setup.py
> +++ b/setup.py
> @@ -47,7 +47,11 @@ x_scripts = {
>  # Dictionary custom modules written in C/C++ here.  The structure is
>  #   key   = module name
>  #   value = list of C/C++ source code, path relative to top source directory
> -x_c_helpers = {}
> +x_c_helpers = {
> +	'portage.util.libc' : [
> +		'src/portage_util_libc.c',
> +	],
> +}
>
>  class x_build(build):
>  	""" Build command with extra build_man call. """
> diff --git a/src/portage_util_libc.c b/src/portage_util_libc.c
> new file mode 100644
> index 0000000..00b09c2
> --- /dev/null
> +++ b/src/portage_util_libc.c
> @@ -0,0 +1,70 @@
> +/* Copyright 2005-2016 Gentoo Foundation
> + * Distributed under the terms of the GNU General Public License v2
> + */
> +
> +#include <Python.h>
> +#include <stdlib.h>
> +#include <ctype.h>
> +
> +static PyObject * _libc_tolower(PyObject *, PyObject *);
> +static PyObject * _libc_toupper(PyObject *, PyObject *);
> +
> +static PyMethodDef LibcMethods[] = {
> +	{"tolower", _libc_tolower, METH_VARARGS, "Convert to lower case using system locale."},
> +	{"toupper", _libc_toupper, METH_VARARGS, "Convert to upper case using system locale."},
> +	{NULL, NULL, 0, NULL}
> +};
> +
> +#if PY_MAJOR_VERSION >= 3
> +static struct PyModuleDef moduledef = {
> +	PyModuleDef_HEAD_INIT,
> +	"libc",								/* m_name */
> +	"Module for converting case using the system locale",		/* m_doc */
> +	-1,								/* m_size */
> +	LibcMethods,							/* m_methods */
> +	NULL,								/* m_reload */
> +	NULL,								/* m_traverse */
> +	NULL,								/* m_clear */
> +	NULL,								/* m_free */
> +};
> +#endif
> +
> +PyMODINIT_FUNC

Now you could call it nitpicking but since it decorates the function,
it would be better to have it inside the #ifdef. Otherwise, people
would think it's separate from that.

> +
> +#if PY_MAJOR_VERSION >= 3
> +PyInit_libc(void)
> +{
> +	PyObject *m;
> +	m = PyModule_Create(&moduledef);
> +	return m;
> +}
> +#else
> +initlibc(void)
> +{
> +	Py_InitModule("libc", LibcMethods);
> +}
> +#endif
> +
> +
> +static PyObject *
> +_libc_tolower(PyObject *self, PyObject *args)
> +{
> +	int c;
> +
> +	if (!PyArg_ParseTuple(args, "i", &c))
> +		return NULL;
> +
> +	return Py_BuildValue("i", tolower(c));
> +}
> +
> +
> +static PyObject *
> +_libc_toupper(PyObject *self, PyObject *args)
> +{
> +	int c;
> +
> +	if (!PyArg_ParseTuple(args, "i", &c))
> +		return NULL;
> +
> +	return Py_BuildValue("i", toupper(c));
> +}

Aside from that, it look nice and shiny now. Thanks a lot!

-- 
Best regards,
Michał Górny
<http://dev.gentoo.org/~mgorny/>