[SSSD] [PATCHES] Add SID related lookups to the NSS responder - part 2

Jakub Hrozek jhrozek at redhat.com
Fri May 10 15:34:27 UTC 2013


On Fri, May 03, 2013 at 06:39:17PM +0200, Sumit Bose wrote:
> I added a 5th patch which adds support for tuples and Unicode to the
> python API as inquired by Alexander. I didn't squeezed them because I'm
> not sure if Alexander was already able to test them. This way they can
> easily be dropped from this release and can be added to the next.
> 
> bye,
> Sumit

The first four patches were pushed upstream already. See below for some
comments about the last patch.

Sorry at first I thought this patch only handled the list input, so I
split Alexander's comments into a separate ticket. I will also mark the
second one (#1914) as assigned to you and set the patch boolean.

> From 4ab0e369d28e3febcc14de2e0801b0e577fc823d Mon Sep 17 00:00:00 2001
> From: Sumit Bose <sbose at redhat.com>
> Date: Fri, 3 May 2013 14:05:23 +0200
> Subject: [PATCH 5/5] Add support for tuples and unicode pysss_nss_idmap.so
> 
> This patch allows tuples as well as lists as input and adds support for
> Unicode objects as input and always returns the results as Unicode
> objects.
> ---
>  src/python/pysss_nss_idmap.c |   64 ++++++++++++++++++++++++++++++-----------
>  1 files changed, 47 insertions(+), 17 deletions(-)
> 
> diff --git a/src/python/pysss_nss_idmap.c b/src/python/pysss_nss_idmap.c
> index dd1fb4c..edd0c00 100644
> --- a/src/python/pysss_nss_idmap.c
> +++ b/src/python/pysss_nss_idmap.c
> @@ -64,6 +64,19 @@ static int add_dict(PyObject *py_result, PyObject *key, PyObject *res_type,
>      return ret;
>  }
>  

I think this function would be a nice addition to src/util/sss_python.c 

> +static char *py_string_or_unicode_as_string(PyObject *inp)
> +{
> +    PyObject *py_str = NULL;
> +
> +    if (PyUnicode_Check(inp)) {
> +        py_str = PyUnicode_AsUTF8String(inp);
> +    } else {
> +        py_str = inp;

Now the function relies on "inp" being a string, would it make sense to
call PyString_Check() here in an else-if branch and raise a TypeError in
the else branch? Maybe with:
    PyErr_Format(PyExc_TypeError, "input must be unicode or a string");

> +    }
> +
> +    return PyString_AS_STRING(py_str);
> +}
> +
>  static int do_getsidbyname(PyObject *py_result, PyObject *py_name)
>  {
>      int ret;
> @@ -71,7 +84,7 @@ static int do_getsidbyname(PyObject *py_result, PyObject *py_name)
>      char *sid = NULL;
>      enum sss_id_type id_type;
>  
> -    name = PyString_AS_STRING(py_name);
> +    name = py_string_or_unicode_as_string(py_name);
>      if (name == NULL) {
>          return EINVAL;
>      }
> @@ -79,7 +92,7 @@ static int do_getsidbyname(PyObject *py_result, PyObject *py_name)
>      ret = sss_nss_getsidbyname(name, &sid, &id_type);
>      if (ret == 0) {
>          ret = add_dict(py_result, py_name, PyString_FromString(SSS_SID_KEY),
> -                       PyString_FromString(sid), PyInt_FromLong(id_type));
> +                       PyUnicode_FromString(sid), PyInt_FromLong(id_type));
>      }
>      free(sid);
>  
> @@ -93,7 +106,7 @@ static int do_getnamebysid(PyObject *py_result, PyObject *py_sid)
>      char *name = NULL;
>      enum sss_id_type id_type;
>  
> -    sid = PyString_AS_STRING(py_sid);
> +    sid = py_string_or_unicode_as_string(py_sid);
>      if (sid == NULL) {
>          return EINVAL;
>      }
> @@ -101,7 +114,7 @@ static int do_getnamebysid(PyObject *py_result, PyObject *py_sid)
>      ret = sss_nss_getnamebysid(sid, &name, &id_type);
>      if (ret == 0) {
>          ret = add_dict(py_result, py_sid, PyString_FromString(SSS_NAME_KEY),
> -                       PyString_FromString(name), PyInt_FromLong(id_type));
> +                       PyUnicode_FromString(name), PyInt_FromLong(id_type));
>      }
>      free(name);
>  
> @@ -121,8 +134,8 @@ static int do_getsidbyid(PyObject *py_result, PyObject *py_id)
>          id = PyInt_AS_LONG(py_id);
>      } else if (PyLong_Check(py_id)) {
>          id = PyLong_AsLong(py_id);
> -    } else if (PyString_Check(py_id)) {
> -        id_str = PyString_AS_STRING(py_id);
> +    } else if (PyString_Check(py_id) || PyUnicode_Check(py_id)) {

If we moved the string check to py_string_or_unicode_as_string() this
check would be obsolete.

> +        id_str = py_string_or_unicode_as_string(py_id);
>          errno = 0;
>          id = strtol(id_str, &endptr, 10);
>          if (errno != 0 || *endptr != '\0') {
> @@ -139,7 +152,7 @@ static int do_getsidbyid(PyObject *py_result, PyObject *py_id)
>      ret = sss_nss_getsidbyid((uint32_t) id, &sid, &id_type);
>      if (ret == 0) {
>          ret = add_dict(py_result, py_id, PyString_FromString(SSS_SID_KEY),
> -                       PyString_FromString(sid), PyInt_FromLong(id_type));
> +                       PyUnicode_FromString(sid), PyInt_FromLong(id_type));
>      }
>      free(sid);
>  
> @@ -153,7 +166,7 @@ static int do_getidbysid(PyObject *py_result, PyObject *py_sid)
>      enum sss_id_type id_type;
>      int ret;
>  
> -    sid = PyString_AS_STRING(py_sid);
> +    sid = py_string_or_unicode_as_string(py_sid);
>      if (sid == NULL) {
>          return EINVAL;
>      }
> @@ -190,22 +203,29 @@ static int do_lookup(enum lookup_type type, PyObject *py_result,
>      return ENOSYS;
>  }
>  
> +typedef Py_ssize_t (*py_lt_size_fn_t)(PyObject *);
> +typedef PyObject *(*py_lt_get_item_fn_t)(PyObject *, Py_ssize_t);

I haven't actually tested it but I think you could use
PySequence_GetItem() and PySequence_Size() instead, I suspect they would
work on both lists and tuples.

>  static PyObject *check_args(enum lookup_type type, PyObject *args)
>  {
>      PyObject *obj, *py_value;
>      int ret;
>      Py_ssize_t len, i;
>      PyObject *py_result;
> +    py_lt_size_fn_t lt_size = NULL;
> +    py_lt_get_item_fn_t lt_get_item = NULL;
> +    bool multival = false;
>  
>      if (!PyArg_ParseTuple(args, sss_py_const_p(char, "O"), &obj)) {
>          PyErr_Format(PyExc_ValueError, "Unable to retrieve argument\n");
>          return NULL;
>      }
>  
> -    if (!(PyList_Check(obj) || PyString_Check(obj) ||
> +    if (!(PyList_Check(obj) || PyTuple_Check(obj) ||
> +          PyString_Check(obj) || PyUnicode_Check(obj) ||
>            (type == SIDBYID && (PyInt_Check(obj) || PyLong_Check(obj))))) {
>          PyErr_Format(PyExc_ValueError,
> -                     "Only string, long or list of them are accepted\n");
> +                     "Only string, long or list or tuples of them " \
> +                     "are accepted\n");
>          return NULL;
>      }
>  
> @@ -218,11 +238,21 @@ static PyObject *check_args(enum lookup_type type, PyObject *args)
>      }
>  
>      if (PyList_Check(obj)) {
> -        len = PyList_Size(obj);
> +        lt_size = PyList_Size;
> +        lt_get_item = PyList_GetItem;
> +        multival = true;
> +    } else if (PyTuple_Check(obj)) {
> +        lt_size = PyTuple_Size;
> +        lt_get_item = PyTuple_GetItem;
> +        multival = true;
> +    }
> +
> +    if (multival) {
> +        len = lt_size(obj);
>          for(i=0; i < len; i++) {
> -            py_value = PyList_GetItem(obj, i);
> +            py_value = lt_get_item(obj, i);
>              if ((py_value != NULL) &&
> -                (PyString_Check(py_value) ||
> +                (PyString_Check(py_value) || PyUnicode_Check(py_value) ||
>                   (type == SIDBYID &&
>                    (PyInt_Check(py_value) || PyLong_Check(py_value))))) {
>                  ret = do_lookup(type, py_result, py_value);



More information about the sssd-devel mailing list