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

[Anthony G. Basile]

On 5/29/16 2:30 AM, Michał Górny wrote:
> 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.

You mean repeat it twice?

>
>> +
>> +#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!

so what's the other stuff that could get put into this module that you
mentioned?

>


-- 
Anthony G. Basile, Ph. D.
Chair of Information Technology
D'Youville College
Buffalo, NY 14201
[phone]