On 07/13/2011 11:57 AM, Alexander Bokovoy wrote:
> On 13.07.2011 12:56, Jakub Hrozek wrote:
>> On 07/13/2011 11:09 AM, Alexander Bokovoy wrote:
>>> Now, there is one concern -- admittedly, my fault as I haven't mention it:
>>> str(true) == 'True'
>>> str(false) == 'False'
>>>
>>> so they should be acceptable as well. You have them explicitly denied in
>>> the unit tests. Do we have any particular reason not to allow them?
>>>
>>> This is minor comment, other than that patches look good!
>>
>> No reason, my previous incarnation of the patch actually had
>> strcasecmp(str, "true") -- would that be preferable? Or would it be
>> better to only explicitly allow for instance true/True/TRUE?
> 
> I'd vote for strcasecmp() way (as in UTF-8 that should be the same as 
> for ASCII). I guess this is the case where some tolerance is good. ;)
> 

Agreed. New patches attached.
From c464c6678e0a51801808c3d1710140e0de42f5c2 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhro...@redhat.com>
Date: Tue, 12 Jul 2011 17:27:33 +0200
Subject: [PATCH 1/2] Fixes for python HBAC bindings

These changes were proposed during a review:
 * Change the signature of str_concat_sequence() to const char *
 * use a getsetter for HbacRule.enabled to allow string true/false and
   integer 1/0 in addition to bool
 * fix a minor memory leak (HbacRequest.rule_name)
 * remove overzealous discard consts
---
 src/python/pyhbac.c      |   94 ++++++++++++++++++++++++++++++++++++++++------
 src/tests/pyhbac-test.py |   23 +++++++++++
 2 files changed, 105 insertions(+), 12 deletions(-)

diff --git a/src/python/pyhbac.c b/src/python/pyhbac.c
index 38d5e7e..85d2f3f 100644
--- a/src/python/pyhbac.c
+++ b/src/python/pyhbac.c
@@ -70,7 +70,7 @@ py_strdup(const char *string)
 }
 
 static char *
-py_strcat_realloc(char *first, char *second)
+py_strcat_realloc(char *first, const char *second)
 {
     char *new_first;
     new_first = PyMem_Realloc(first, strlen(first) + strlen(second) + 1);
@@ -229,7 +229,7 @@ native_category(PyObject *pycat)
 }
 
 static char *
-str_concat_sequence(PyObject *seq, char *delim)
+str_concat_sequence(PyObject *seq, const char *delim)
 {
     Py_ssize_t size;
     Py_ssize_t i;
@@ -284,7 +284,7 @@ static void
 set_hbac_exception(PyObject *exc, struct hbac_info *error)
 {
     PyErr_SetObject(exc,
-                    Py_BuildValue(discard_const_p(char, "(i,s)"),
+                    Py_BuildValue("(i,s)",
                                   error->code,
                                   error->rule_name ? \
                                         error->rule_name : "no rule"));
@@ -367,7 +367,7 @@ HbacRuleElement_init(HbacRuleElement *self, PyObject *args, PyObject *kwargs)
     PyObject *tmp = NULL;
 
     if (!PyArg_ParseTupleAndKeywords(args, kwargs,
-                                     discard_const_p(char, "|OOO"),
+                                     "|OOO",
                                      discard_const_p(char *, kwlist),
                                      &names, &groups, &category)) {
         return -1;
@@ -711,7 +711,7 @@ HbacRule_init(HbacRuleObject *self, PyObject *args, PyObject *kwargs)
     PyObject *empty_tuple = NULL;
 
     if (!PyArg_ParseTupleAndKeywords(args, kwargs,
-                                     discard_const_p(char, "O|i"),
+                                     "O|i",
                                      discard_const_p(char *, kwlist),
                                      &name, &self->enabled)) {
         return -1;
@@ -739,6 +739,73 @@ HbacRule_init(HbacRuleObject *self, PyObject *args, PyObject *kwargs)
 }
 
 static int
+hbac_rule_set_enabled(HbacRuleObject *self, PyObject *enabled, void *closure)
+{
+    CHECK_ATTRIBUTE_DELETE(enabled, "enabled");
+
+    if (PyString_Check(enabled) || PyUnicode_Check(enabled)) {
+        PyObject *utf8_str;
+        char *str;
+
+        utf8_str = get_utf8_string(enabled, "enabled");
+        if (!utf8_str) return -1;
+        str = PyString_AsString(utf8_str);
+        if (!str) {
+            Py_DECREF(utf8_str);
+            return -1;
+        }
+
+        if (strcasecmp(str, "true") == 0) {
+            self->enabled = true;
+        } else if (strcasecmp(str, "false") == 0) {
+            self->enabled = false;
+        } else {
+            PyErr_Format(PyExc_ValueError,
+                         "enabled only accepts 'true' of 'false' "
+                         "string literals");
+            Py_DECREF(utf8_str);
+            return -1;
+        }
+
+        Py_DECREF(utf8_str);
+        return 0;
+    } else if (PyBool_Check(enabled)) {
+        self->enabled = (enabled == Py_True);
+        return 0;
+    } else if (PyInt_Check(enabled)) {
+        switch(PyInt_AsLong(enabled)) {
+            case 0:
+                self->enabled = false;
+                break;
+            case 1:
+                self->enabled = true;
+                break;
+            default:
+                PyErr_Format(PyExc_ValueError,
+                            "enabled only accepts '0' of '1' "
+                            "integer constants");
+                return -1;
+        }
+        return 0;
+    }
+
+    PyErr_Format(PyExc_TypeError, "enabled must be a boolean, an integer "
+                                  "1 or 0 or a string constant true/false");
+    return -1;
+
+}
+
+static PyObject *
+hbac_rule_get_enabled(HbacRuleObject *self, void *closure)
+{
+    if (self->enabled) {
+        Py_RETURN_TRUE;
+    }
+
+    Py_RETURN_FALSE;
+}
+
+static int
 hbac_rule_set_name(HbacRuleObject *self, PyObject *name, void *closure)
 {
     CHECK_ATTRIBUTE_DELETE(name, "name");
@@ -811,8 +878,6 @@ HbacRule_repr(HbacRuleObject *self)
     return o;
 }
 
-PyDoc_STRVAR(HbacRuleObject_enabled__doc__,
-"(bool) Is the rule enabled");
 PyDoc_STRVAR(HbacRuleObject_users__doc__,
 "(HbacRuleElement) Users and user groups for which this rule applies");
 PyDoc_STRVAR(HbacRuleObject_services__doc__,
@@ -823,10 +888,6 @@ PyDoc_STRVAR(HbacRuleObject_srchosts__doc__,
 "(HbacRuleElement) Source hosts for which this rule applies");
 
 static PyMemberDef py_hbac_rule_members[] = {
-    { discard_const_p(char, "enabled"), T_BOOL,
-      offsetof(HbacRuleObject, enabled), 0,
-      HbacRuleObject_enabled__doc__ },
-
     { discard_const_p(char, "users"), T_OBJECT_EX,
       offsetof(HbacRuleObject, users), 0,
       HbacRuleObject_users__doc__ },
@@ -846,10 +907,18 @@ static PyMemberDef py_hbac_rule_members[] = {
     { NULL, 0, 0, 0, NULL } /* Sentinel */
 };
 
+PyDoc_STRVAR(HbacRuleObject_enabled__doc__,
+"(bool) Is the rule enabled");
 PyDoc_STRVAR(HbacRuleObject_name__doc__,
 "(string) The name of the rule");
 
 static PyGetSetDef py_hbac_rule_getset[] = {
+    { discard_const_p(char, "enabled"),
+      (getter) hbac_rule_get_enabled,
+      (setter) hbac_rule_set_enabled,
+      HbacRuleObject_enabled__doc__,
+      NULL },
+
     { discard_const_p(char, "name"),
       (getter) hbac_rule_get_name,
       (setter) hbac_rule_set_name,
@@ -1023,7 +1092,7 @@ HbacRequestElement_init(HbacRequestElement *self,
     PyObject *groups = NULL;
 
     if (!PyArg_ParseTupleAndKeywords(args, kwargs,
-                                     discard_const_p(char, "|OO"),
+                                     "|OO",
                                      discard_const_p(char *, kwlist),
                                      &name, &groups)) {
         return -1;
@@ -1272,6 +1341,7 @@ HbacRequest_clear(HbacRequest *self)
     Py_CLEAR(self->user);
     Py_CLEAR(self->targethost);
     Py_CLEAR(self->srchost);
+    Py_CLEAR(self->rule_name);
     return 0;
 }
 
diff --git a/src/tests/pyhbac-test.py b/src/tests/pyhbac-test.py
index fdf4ac3..b15d165 100755
--- a/src/tests/pyhbac-test.py
+++ b/src/tests/pyhbac-test.py
@@ -137,8 +137,31 @@ class PyHbacRuleTest(unittest.TestCase):
         rule.enabled = False
         self.assertEqual(rule.enabled, False)
 
+        rule.enabled = "TRUE"
+        self.assertEqual(rule.enabled, True)
+        rule.enabled = "FALSE"
+        self.assertEqual(rule.enabled, False)
+
+        rule.enabled = "true"
+        self.assertEqual(rule.enabled, True)
+        rule.enabled = "false"
+        self.assertEqual(rule.enabled, False)
+
+        rule.enabled = "True"
+        self.assertEqual(rule.enabled, True)
+        rule.enabled = "False"
+        self.assertEqual(rule.enabled, False)
+
+        rule.enabled = 1
+        self.assertEqual(rule.enabled, True)
+        rule.enabled = 0
+        self.assertEqual(rule.enabled, False)
+
         # negative test
         self.assertRaises(TypeError, rule.__setattr__, "enabled", None)
+        self.assertRaises(TypeError, rule.__setattr__, "enabled", [])
+        self.assertRaises(ValueError, rule.__setattr__, "enabled", "foo")
+        self.assertRaises(ValueError, rule.__setattr__, "enabled", 5)
 
     def testRuleElementInRule(self):
         users = [ "foo", "bar" ]
-- 
1.7.5.4

From 1a02ea171ef6fa3a9834549374e700df84dae74f Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhro...@redhat.com>
Date: Tue, 12 Jul 2011 21:22:22 +0200
Subject: [PATCH 2/2] Fix python HBAC bindings for python <= 2.4

Several parts of the HBAC python bindings did not work with old Python
versions, such as the one shipped in RHEL5.

The changes include:
* a compatibility wrapper around python set object
* PyModule_AddIntMacro compat macro
* Py_ssize_t compat definition
* Do not use PyUnicode_FromFormat
* several function prototypes and structures used to have "char
  arguments where they have "const char *" in recent versions.
  This caused compilation warnings this patch mitigates by using
  the discard_const hack on python 2.4
---
 Makefile.am              |    3 +-
 configure.ac             |    1 +
 src/external/python.m4   |   20 +++++
 src/python/pyhbac.c      |  200 +++++++++++++++++++++++++++-------------------
 src/tests/pyhbac-test.py |    7 +-
 src/util/sss_python.c    |  104 ++++++++++++++++++++++++
 src/util/sss_python.h    |   63 +++++++++++++++
 7 files changed, 313 insertions(+), 85 deletions(-)
 create mode 100644 src/util/sss_python.c
 create mode 100644 src/util/sss_python.h

diff --git a/Makefile.am b/Makefile.am
index 1eb2e7a..2d54a8e 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -1019,7 +1019,8 @@ pysss_la_LDFLAGS = \
     -module
 
 pyhbac_la_SOURCES = \
-    src/python/pyhbac.c
+    src/python/pyhbac.c \
+    src/util/sss_python.c
 pyhbac_la_CFLAGS = \
     $(AM_CFLAGS)  \
     $(PYTHON_CFLAGS)
diff --git a/configure.ac b/configure.ac
index 2cf3137..0e2e6d4 100644
--- a/configure.ac
+++ b/configure.ac
@@ -150,6 +150,7 @@ if test x$HAVE_PYTHON_BINDINGS != x; then
     AM_CHECK_PYTHON_HEADERS([],
                             AC_MSG_ERROR([Could not find python headers]))
     AM_PYTHON_CONFIG
+    AM_CHECK_PYTHON_COMPAT
 fi
 
 if test x$HAVE_SELINUX != x; then
diff --git a/src/external/python.m4 b/src/external/python.m4
index db59863..00487a7 100644
--- a/src/external/python.m4
+++ b/src/external/python.m4
@@ -56,3 +56,23 @@ AC_DEFUN([AM_CHECK_PYTHON_HEADERS],
 ])
 
 
+dnl Checks for a couple of functions we use that may not be defined
+dnl in some older python versions used e.g. on RHEL5
+AC_DEFUN([AM_CHECK_PYTHON_COMPAT],
+[AC_REQUIRE([AM_CHECK_PYTHON_HEADERS])
+    save_CPPFLAGS="$CPPFLAGS"
+    save_LIBS="$LIBS"
+    CPPFLAGS="$CPPFLAGS $PYTHON_INCLUDES"
+    LIBS="$LIBS $PYTHON_LIBS"
+
+    AC_CHECK_TYPE(Py_ssize_t,
+                  [ AC_DEFINE_UNQUOTED(HAVE_PY_SSIZE_T, 1, [Native Py_ssize_t type]) ],
+                  [],
+                  [[#include <Python.h>]])
+
+    AC_CHECK_FUNCS([PySet_New PySet_Add PyErr_NewExceptionWithDoc])
+    AC_CHECK_DECLS([PySet_Check, PyModule_AddIntMacro, PyUnicode_FromString], [], [], [[#include <Python.h>]])
+
+    CPPFLAGS="$save_CPPFLAGS"
+    LIBS="$save_LIBS"
+])
diff --git a/src/python/pyhbac.c b/src/python/pyhbac.c
index 85d2f3f..97c149a 100644
--- a/src/python/pyhbac.c
+++ b/src/python/pyhbac.c
@@ -22,18 +22,16 @@
 #include <structmember.h>
 
 #include "util/util.h"
+#include "util/sss_python.h"
 #include "providers/ipa/ipa_hbac.h"
 
 #define PYTHON_MODULE_NAME  "pyhbac"
 
-#define TYPE_READY(module, type, name) do {         \
-    if (PyType_Ready(&type) < 0)                    \
-        return;                                     \
-    Py_INCREF(&type);                               \
-    PyModule_AddObject(module,                      \
-                       discard_const_p(char, name), \
-                       (PyObject *) &type);         \
-} while(0);                                         \
+#ifndef PYHBAC_ENCODING
+#define PYHBAC_ENCODING "UTF-8"
+#endif
+
+#define PYHBAC_ENCODING_ERRORS "strict"
 
 #define CHECK_ATTRIBUTE_DELETE(attr, attrname) do {         \
     if (attr == NULL) {                                     \
@@ -44,14 +42,6 @@
     }                                                       \
 } while(0);
 
-#define SAFE_SET(old, new) do {         \
-    PyObject *__simple_set_tmp = NULL;  \
-    __simple_set_tmp = old;             \
-    Py_INCREF(new);                     \
-    old = new;                          \
-    Py_XDECREF(__simple_set_tmp);       \
-} while(0);
-
 static PyObject *PyExc_HbacError;
 
 /* ==================== Utility functions ========================*/
@@ -284,7 +274,7 @@ static void
 set_hbac_exception(PyObject *exc, struct hbac_info *error)
 {
     PyErr_SetObject(exc,
-                    Py_BuildValue("(i,s)",
+                    Py_BuildValue(sss_py_const_p(char, "(i,s)"),
                                   error->code,
                                   error->rule_name ? \
                                         error->rule_name : "no rule"));
@@ -310,7 +300,7 @@ HbacRuleElement_new(PyTypeObject *type, PyObject *args, PyObject *kwds)
         return NULL;
     }
 
-    self->category = PySet_New(NULL);
+    self->category = sss_python_set_new();
     self->names = PyList_New(0);
     self->groups = PyList_New(0);
     if (!self->names || !self->groups || !self->category) {
@@ -367,7 +357,7 @@ HbacRuleElement_init(HbacRuleElement *self, PyObject *args, PyObject *kwargs)
     PyObject *tmp = NULL;
 
     if (!PyArg_ParseTupleAndKeywords(args, kwargs,
-                                     "|OOO",
+                                     sss_py_const_p(char, "|OOO"),
                                      discard_const_p(char *, kwlist),
                                      &names, &groups, &category)) {
         return -1;
@@ -395,7 +385,7 @@ HbacRuleElement_init(HbacRuleElement *self, PyObject *args, PyObject *kwargs)
             return -1;
         }
 
-        if (PySet_Add(self->category, tmp) != 0) {
+        if (sss_python_set_add(self->category, tmp) != 0) {
             return -1;
         }
     }
@@ -458,7 +448,7 @@ hbac_rule_element_set_category(HbacRuleElement *self,
 
     CHECK_ATTRIBUTE_DELETE(category, "category");
 
-    if (!PySet_Check(category)) {
+    if (!sss_python_set_check(category)) {
         PyErr_Format(PyExc_TypeError, "The category must be a set type\n");
         return -1;
     }
@@ -497,7 +487,12 @@ HbacRuleElement_repr(HbacRuleElement *self)
     char *strnames = NULL;
     char *strgroups = NULL;
     uint32_t category;
-    PyObject *o;
+    PyObject *o, *format, *args;
+
+    format = sss_python_unicode_from_string("<category %lu names [%s] groups [%s]>");
+    if (format == NULL) {
+        return NULL;
+    }
 
     strnames = str_concat_sequence(self->names,
                                    discard_const_p(char, ","));
@@ -507,13 +502,24 @@ HbacRuleElement_repr(HbacRuleElement *self)
     if (strnames == NULL || strgroups == NULL || category == -1) {
         PyMem_Free(strnames);
         PyMem_Free(strgroups);
+        Py_DECREF(format);
+        return NULL;
+    }
+
+    args = Py_BuildValue(sss_py_const_p(char, "Kss"),
+                         category, strnames, strgroups);
+    if (args == NULL) {
+        PyMem_Free(strnames);
+        PyMem_Free(strgroups);
+        Py_DECREF(format);
         return NULL;
     }
 
-    o = PyUnicode_FromFormat("<category %lu names [%s] groups [%s]>",
-                             category, strnames, strgroups);
+    o = PyUnicode_Format(format, args);
     PyMem_Free(strnames);
     PyMem_Free(strgroups);
+    Py_DECREF(format);
+    Py_DECREF(args);
     return o;
 }
 
@@ -554,7 +560,7 @@ PyDoc_STRVAR(HbacRuleElement__doc__,
 
 static PyTypeObject pyhbac_hbacrule_element_type = {
     PyObject_HEAD_INIT(NULL)
-    .tp_name = "pyhbac.HbacRuleElement",
+    .tp_name = sss_py_const_p(char, "pyhbac.HbacRuleElement"),
     .tp_basicsize = sizeof(HbacRuleElement),
     .tp_new = HbacRuleElement_new,
     .tp_dealloc = (destructor) HbacRuleElement_dealloc,
@@ -635,7 +641,7 @@ HbacRule_new(PyTypeObject *type, PyObject *args, PyObject *kwds)
         return NULL;
     }
 
-    self->name = PyUnicode_FromString("");
+    self->name = sss_python_unicode_from_string("");
     if (self->name == NULL) {
         Py_DECREF(self);
         PyErr_NoMemory();
@@ -692,11 +698,11 @@ HbacRule_dealloc(HbacRuleObject *self)
 static int
 HbacRule_traverse(HbacRuleObject *self, visitproc visit, void *arg)
 {
-    Py_VISIT(self->name);
-    Py_VISIT(self->services);
-    Py_VISIT(self->users);
-    Py_VISIT(self->targethosts);
-    Py_VISIT(self->srchosts);
+    Py_VISIT((PyObject *) self->name);
+    Py_VISIT((PyObject *) self->services);
+    Py_VISIT((PyObject *) self->users);
+    Py_VISIT((PyObject *) self->targethosts);
+    Py_VISIT((PyObject *) self->srchosts);
     return 0;
 }
 
@@ -711,7 +717,7 @@ HbacRule_init(HbacRuleObject *self, PyObject *args, PyObject *kwargs)
     PyObject *empty_tuple = NULL;
 
     if (!PyArg_ParseTupleAndKeywords(args, kwargs,
-                                     "O|i",
+                                     sss_py_const_p(char, "O|i"),
                                      discard_const_p(char *, kwlist),
                                      &name, &self->enabled)) {
         return -1;
@@ -826,7 +832,8 @@ hbac_rule_get_name(HbacRuleObject *self, void *closure)
         Py_INCREF(self->name);
         return self->name;
     } else if (PyString_Check(self->name)) {
-        return PyUnicode_FromObject(self->name);
+        return PyUnicode_FromEncodedObject(self->name,
+                                           PYHBAC_ENCODING, PYHBAC_ENCODING_ERRORS);
     }
 
     /* setter does typechecking but let us be paranoid */
@@ -837,15 +844,16 @@ hbac_rule_get_name(HbacRuleObject *self, void *closure)
 static PyObject *
 HbacRule_repr(HbacRuleObject *self)
 {
-    PyObject *utf_name;
     PyObject *users_repr;
     PyObject *services_repr;
     PyObject *targethosts_repr;
     PyObject *srchosts_repr;
-    PyObject *o;
+    PyObject *o, *format, *args;
 
-    utf_name = get_utf8_string(self->name, "name");
-    if (utf_name == NULL) {
+    format = sss_python_unicode_from_string("<name %s enabled %d "
+                                            "users %s services %s "
+                                            "targethosts %s srchosts %s>");
+    if (format == NULL) {
         return NULL;
     }
 
@@ -855,26 +863,34 @@ HbacRule_repr(HbacRuleObject *self)
     srchosts_repr = HbacRuleElement_repr(self->srchosts);
     if (users_repr == NULL || services_repr == NULL ||
         targethosts_repr == NULL || srchosts_repr == NULL) {
-        Py_DECREF(utf_name);
         Py_XDECREF(users_repr);
         Py_XDECREF(services_repr);
         Py_XDECREF(targethosts_repr);
         Py_XDECREF(srchosts_repr);
+        Py_DECREF(format);
         return NULL;
     }
 
-    o = PyUnicode_FromFormat("<name %s enabled %d "
-                             "users %U services %U "
-                             "targethosts %U srchosts %U>",
-                             PyString_AsString(utf_name),
-                             self->enabled,
-                             users_repr, services_repr,
-                             targethosts_repr, srchosts_repr);
-    Py_DECREF(utf_name);
+    args = Py_BuildValue(sss_py_const_p(char, "OiOOOO"),
+                         self->name, self->enabled,
+                         users_repr, services_repr,
+                         targethosts_repr, srchosts_repr);
+    if (args == NULL) {
+        Py_DECREF(users_repr);
+        Py_DECREF(services_repr);
+        Py_DECREF(targethosts_repr);
+        Py_DECREF(srchosts_repr);
+        Py_DECREF(format);
+        return NULL;
+    }
+
+    o = PyUnicode_Format(format, args);
     Py_DECREF(users_repr);
     Py_DECREF(services_repr);
     Py_DECREF(targethosts_repr);
     Py_DECREF(srchosts_repr);
+    Py_DECREF(format);
+    Py_DECREF(args);
     return o;
 }
 
@@ -937,7 +953,7 @@ PyDoc_STRVAR(HbacRuleObject__doc__,
 
 static PyTypeObject pyhbac_hbacrule_type = {
     PyObject_HEAD_INIT(NULL)
-    .tp_name = "pyhbac.HbacRule",
+    .tp_name = sss_py_const_p(char, "pyhbac.HbacRule"),
     .tp_basicsize = sizeof(HbacRuleObject),
     .tp_new = HbacRule_new,
     .tp_dealloc = (destructor) HbacRule_dealloc,
@@ -1031,7 +1047,7 @@ HbacRequestElement_new(PyTypeObject *type, PyObject *args, PyObject *kwds)
         return NULL;
     }
 
-    self->name = PyUnicode_FromString("");
+    self->name = sss_python_unicode_from_string("");
     if (self->name == NULL) {
         PyErr_NoMemory();
         Py_DECREF(self);
@@ -1092,7 +1108,7 @@ HbacRequestElement_init(HbacRequestElement *self,
     PyObject *groups = NULL;
 
     if (!PyArg_ParseTupleAndKeywords(args, kwargs,
-                                     "|OO",
+                                     sss_py_const_p(char, "|OO"),
                                      discard_const_p(char *, kwlist),
                                      &name, &groups)) {
         return -1;
@@ -1136,7 +1152,8 @@ hbac_request_element_get_name(HbacRequestElement *self, void *closure)
         Py_INCREF(self->name);
         return self->name;
     } else if (PyString_Check(self->name)) {
-        return PyUnicode_FromObject(self->name);
+        return PyUnicode_FromEncodedObject(self->name,
+                                           PYHBAC_ENCODING, PYHBAC_ENCODING_ERRORS);
     }
 
     /* setter does typechecking but let us be paranoid */
@@ -1169,25 +1186,28 @@ hbac_request_element_get_groups(HbacRequestElement *self, void *closure)
 static PyObject *
 HbacRequestElement_repr(HbacRequestElement *self)
 {
-    PyObject *utf_name;
     char *strgroups;
-    PyObject *o;
+    PyObject *o, *format, *args;
 
-    utf_name = get_utf8_string(self->name, "name");
-    if (utf_name == NULL) {
+    format = sss_python_unicode_from_string("<name %s groups [%s]>");
+    if (format == NULL) {
         return NULL;
     }
 
     strgroups = str_concat_sequence(self->groups, discard_const_p(char, ","));
     if (strgroups == NULL) {
-        Py_DECREF(utf_name);
+        Py_DECREF(format);
         return NULL;
     }
 
-    o = PyUnicode_FromFormat("<name %s groups [%s]>",
-                             PyString_AsString(utf_name), strgroups);
-    Py_DECREF(utf_name);
+    args = Py_BuildValue(sss_py_const_p(char, "Os"), self->name, strgroups);
+    if (args == NULL) {
+    }
+
+    o = PyUnicode_Format(format, args);
     PyMem_Free(strgroups);
+    Py_DECREF(format);
+    Py_DECREF(args);
     return o;
 }
 
@@ -1220,7 +1240,7 @@ PyDoc_STRVAR(HbacRequestElement__doc__,
 
 static PyTypeObject pyhbac_hbacrequest_element_type = {
     PyObject_HEAD_INIT(NULL)
-    .tp_name = "pyhbac.HbacRequestElement",
+    .tp_name = sss_py_const_p(char, "pyhbac.HbacRequestElement"),
     .tp_basicsize = sizeof(HbacRequestElement),
     .tp_new = HbacRequestElement_new,
     .tp_dealloc = (destructor) HbacRequestElement_dealloc,
@@ -1355,10 +1375,10 @@ HbacRequest_dealloc(HbacRequest *self)
 static int
 HbacRequest_traverse(HbacRequest *self, visitproc visit, void *arg)
 {
-    Py_VISIT(self->service);
-    Py_VISIT(self->user);
-    Py_VISIT(self->targethost);
-    Py_VISIT(self->srchost);
+    Py_VISIT((PyObject *) self->service);
+    Py_VISIT((PyObject *) self->user);
+    Py_VISIT((PyObject *) self->targethost);
+    Py_VISIT((PyObject *) self->srchost);
     return 0;
 }
 
@@ -1430,7 +1450,7 @@ py_hbac_evaluate(HbacRequest *self, PyObject *args)
     PyObject *ret = NULL;
     long i;
 
-    if (!PyArg_ParseTuple(args, "O", &py_rules_list)) {
+    if (!PyArg_ParseTuple(args, sss_py_const_p(char, "O"), &py_rules_list)) {
         goto fail;
     }
 
@@ -1484,7 +1504,7 @@ py_hbac_evaluate(HbacRequest *self, PyObject *args)
     eres = hbac_evaluate(rules, hbac_req, &info);
     switch (eres) {
     case HBAC_EVAL_ALLOW:
-        self->rule_name = PyUnicode_FromString(info->rule_name);
+        self->rule_name = sss_python_unicode_from_string(info->rule_name);
         if (!self->rule_name) {
             PyErr_NoMemory();
             goto fail;
@@ -1535,7 +1555,13 @@ HbacRequest_repr(HbacRequest *self)
     PyObject *service_repr;
     PyObject *targethost_repr;
     PyObject *srchost_repr;
-    PyObject *o;
+    PyObject *o, *format, *args;
+
+    format = sss_python_unicode_from_string("<user %s service %s "
+                                            "targethost %s srchost %s>");
+    if (format == NULL) {
+        return NULL;
+    }
 
     user_repr = HbacRequestElement_repr(self->user);
     service_repr = HbacRequestElement_repr(self->service);
@@ -1547,22 +1573,34 @@ HbacRequest_repr(HbacRequest *self)
         Py_XDECREF(service_repr);
         Py_XDECREF(targethost_repr);
         Py_XDECREF(srchost_repr);
+        Py_DECREF(format);
         return NULL;
     }
 
-    o = PyUnicode_FromFormat("<user %U service %U "
-                             "targethost %U srchost %U>",
-                             user_repr, service_repr,
-                             targethost_repr, srchost_repr);
+    args = Py_BuildValue(sss_py_const_p(char, "OOOO"),
+                         user_repr, service_repr,
+                         targethost_repr, srchost_repr);
+    if (args == NULL) {
+        Py_DECREF(user_repr);
+        Py_DECREF(service_repr);
+        Py_DECREF(targethost_repr);
+        Py_DECREF(srchost_repr);
+        Py_DECREF(format);
+    }
+
+    o = PyUnicode_Format(format, args);
     Py_DECREF(user_repr);
     Py_DECREF(service_repr);
     Py_DECREF(targethost_repr);
     Py_DECREF(srchost_repr);
+    Py_DECREF(format);
+    Py_DECREF(args);
     return o;
 }
 
 static PyMethodDef py_hbac_request_methods[] = {
-    { "evaluate", (PyCFunction) py_hbac_evaluate,
+    { sss_py_const_p(char, "evaluate"),
+      (PyCFunction) py_hbac_evaluate,
       METH_VARARGS, py_hbac_evaluate__doc__
     },
     { NULL, NULL, 0, NULL }        /* Sentinel */
@@ -1626,7 +1664,7 @@ PyDoc_STRVAR(HbacRequest__doc__,
 
 static PyTypeObject pyhbac_hbacrequest_type = {
     PyObject_HEAD_INIT(NULL)
-    .tp_name = "pyhbac.HbacRequest",
+    .tp_name = sss_py_const_p(char, "pyhbac.HbacRequest"),
     .tp_basicsize = sizeof(HbacRequest),
     .tp_new = HbacRequest_new,
     .tp_dealloc = (destructor) HbacRequest_dealloc,
@@ -1698,7 +1736,7 @@ py_hbac_result_string(PyObject *module, PyObject *args)
     enum hbac_eval_result result;
     const char *str;
 
-    if (!PyArg_ParseTuple(args, "i", &result)) {
+    if (!PyArg_ParseTuple(args, sss_py_const_p(char, "i"), &result)) {
         return NULL;
     }
 
@@ -1709,7 +1747,7 @@ py_hbac_result_string(PyObject *module, PyObject *args)
         return Py_None;
     }
 
-    return PyUnicode_FromString(str);
+    return sss_python_unicode_from_string(str);
 }
 
 PyDoc_STRVAR(py_hbac_error_string__doc__,
@@ -1722,7 +1760,7 @@ py_hbac_error_string(PyObject *module, PyObject *args)
     enum hbac_error_code code;
     const char *str;
 
-    if (!PyArg_ParseTuple(args, "i", &code)) {
+    if (!PyArg_ParseTuple(args, sss_py_const_p(char, "i"), &code)) {
         return NULL;
     }
 
@@ -1733,17 +1771,17 @@ py_hbac_error_string(PyObject *module, PyObject *args)
         return Py_None;
     }
 
-    return PyUnicode_FromString(str);
+    return sss_python_unicode_from_string(str);
 }
 
 static PyMethodDef pyhbac_module_methods[] = {
-        { "hbac_result_string",
+        {  sss_py_const_p(char, "hbac_result_string"),
            (PyCFunction) py_hbac_result_string,
            METH_VARARGS,
            py_hbac_result_string__doc__,
         },
 
-        { "hbac_error_string",
+        { sss_py_const_p(char, "hbac_error_string"),
            (PyCFunction) py_hbac_error_string,
            METH_VARARGS,
            py_hbac_error_string__doc__,
@@ -1766,16 +1804,16 @@ initpyhbac(void)
     PyObject *m;
     int ret;
 
-    m = Py_InitModule(PYTHON_MODULE_NAME, pyhbac_module_methods);
+    m = Py_InitModule(sss_py_const_p(char, PYTHON_MODULE_NAME), pyhbac_module_methods);
     if (m == NULL) return;
 
     /* The HBAC module exception */
-    PyExc_HbacError = PyErr_NewExceptionWithDoc(
+    PyExc_HbacError = sss_exception_with_doc(
                         discard_const_p(char, "hbac.HbacError"),
                         HbacError__doc__,
                         PyExc_EnvironmentError, NULL);
     Py_INCREF(PyExc_HbacError);
-    ret = PyModule_AddObject(m, "HbacError", PyExc_HbacError);
+    ret = PyModule_AddObject(m, sss_py_const_p(char, "HbacError"), PyExc_HbacError);
     if (ret == -1) return;
 
     /* HBAC rule categories */
diff --git a/src/tests/pyhbac-test.py b/src/tests/pyhbac-test.py
index b15d165..b19fe58 100755
--- a/src/tests/pyhbac-test.py
+++ b/src/tests/pyhbac-test.py
@@ -11,10 +11,10 @@ if not srcdir:
 MODPATH = srcdir + "/.libs" #FIXME - is there a way to get this from libtool?
 
 def compat_assertItemsEqual(this, expected_seq, actual_seq, msg=None):
-    return self.assertEqual(sorted(expected_seq), sorted(actual_seq))
+    return this.assertEqual(sorted(expected_seq), sorted(actual_seq))
 
 def compat_assertIsInstance(this, obj, cls, msg=None):
-    return self.assertTrue(isinstance(obj, cls))
+    return this.assertTrue(isinstance(obj, cls))
 
 # add compat methods for old unittest.TestCase versions
 # (python < 2.7, RHEL5 for instance)
@@ -312,7 +312,8 @@ class PyHbacRequestTest(unittest.TestCase):
     def testRuleName(self):
         req = pyhbac.HbacRequest()
         self.assertEqual(req.rule_name, None)
-        self.assertRaises(AttributeError, req.__setattr__, "rule_name", "foo")
+        # python 2.4 raises TypError, 2.7 raises AttributeError
+        self.assertRaises((TypeError, AttributeError), req.__setattr__, "rule_name", "foo")
 
     def testEvaluate(self):
         name = "someuser"
diff --git a/src/util/sss_python.c b/src/util/sss_python.c
new file mode 100644
index 0000000..19717a5
--- /dev/null
+++ b/src/util/sss_python.c
@@ -0,0 +1,104 @@
+/*
+    Authors:
+        Jakub Hrozek <jhro...@redhat.com>
+
+    Copyright (C) 2011 Red Hat
+
+    This program is free software; you can redistribute it and/or modify
+    it under the terms of the GNU General Public License as published by
+    the Free Software Foundation; either version 3 of the License, or
+    (at your option) any later version.
+
+    This program is distributed in the hope that it will be useful,
+    but WITHOUT ANY WARRANTY; without even the implied warranty of
+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+    GNU General Public License for more details.
+
+    You should have received a copy of the GNU General Public License
+    along with this program.  If not, see <http://www.gnu.org/licenses/>.
+*/
+
+#include "src/util/sss_python.h"
+#include "config.h"
+
+PyObject *
+sss_python_set_new(void)
+{
+#ifdef HAVE_PYSET_NEW
+    return PySet_New(NULL);
+#else
+    return PyObject_CallObject((PyObject *) &PySet_Type, NULL);
+#endif
+}
+
+int
+sss_python_set_add(PyObject *set, PyObject *key)
+{
+#ifdef HAVE_PYSET_ADD
+    return PySet_Add(set, key);
+#else
+    PyObject *pyret;
+    int ret;
+
+    pyret = PyObject_CallMethod(set, sss_py_const_p(char, "add"),
+                                sss_py_const_p(char, "O"), key);
+    ret = (pyret == NULL) ? -1 : 0;
+    Py_XDECREF(pyret);
+    return ret;
+#endif
+}
+
+bool
+sss_python_set_check(PyObject *set)
+{
+#if HAVE_DECL_PYSET_CHECK
+    return PySet_Check(set);
+#else
+    return PyObject_TypeCheck(set, &PySet_Type);
+#endif
+}
+
+PyObject *
+sss_python_unicode_from_string(const char *u)
+{
+#ifdef HAVE_PYUNICODE_FROMSTRING
+    return PyUnicode_FromString(u);
+#else
+    return PyUnicode_DecodeUTF8(u, strlen(u), NULL);
+#endif
+}
+
+PyObject *
+sss_exception_with_doc(char *name, char *doc, PyObject *base, PyObject *dict)
+{
+#ifdef HAVE_PYERR_NEWEXCEPTIONWITHDOC
+    return PyErr_NewExceptionWithDoc(name, doc, base, dict);
+#else
+    int result;
+    PyObject *ret = NULL;
+    PyObject *mydict = NULL; /* points to the dict only if we create it */
+    PyObject *docobj;
+
+    if (dict == NULL) {
+        dict = mydict = PyDict_New();
+        if (dict == NULL) {
+            return NULL;
+        }
+    }
+
+    if (doc != NULL) {
+        docobj = PyString_FromString(doc);
+        if (docobj == NULL)
+            goto failure;
+        result = PyDict_SetItemString(dict, "__doc__", docobj);
+        Py_DECREF(docobj);
+        if (result < 0)
+            goto failure;
+    }
+
+    ret = PyErr_NewException(name, base, dict);
+  failure:
+    Py_XDECREF(mydict);
+    return ret;
+#endif
+}
diff --git a/src/util/sss_python.h b/src/util/sss_python.h
new file mode 100644
index 0000000..135c287
--- /dev/null
+++ b/src/util/sss_python.h
@@ -0,0 +1,63 @@
+#ifndef __SSS_PYTHON_H__
+#define __SSS_PYTHON_H__
+
+#include <Python.h>
+#include <stdbool.h>
+#include "util/util.h"
+
+#if PY_VERSION_HEX < 0x02050000
+#define sss_py_const_p(type, value) discard_const_p(type, (value))
+#else
+#define sss_py_const_p(type, value) (value)
+#endif
+
+/* Py_ssize_t compatibility for python < 2.5 as per
+ * http://www.python.org/dev/peps/pep-0353/ */
+#ifndef HAVE_PY_SSIZE_T
+typedef int Py_ssize_t;
+#endif
+
+#ifndef PY_SSIZE_T_MAX
+#define PY_SSIZE_T_MAX INT_MAX
+#endif
+
+#ifndef PY_SSIZE_T_MIN
+#define PY_SSIZE_T_MIN INT_MIN
+#endif
+
+/* Wrappers providing the subset of C API for python's set objects we use */
+PyObject *sss_python_set_new(void);
+int sss_python_set_add(PyObject *set, PyObject *key);
+bool sss_python_set_check(PyObject *set);
+
+/* Unicode compatibility */
+PyObject *sss_python_unicode_from_string(const char *u);
+
+/* Exceptions compatibility */
+PyObject *
+sss_exception_with_doc(char *name, char *doc, PyObject *base, PyObject *dict);
+
+/* PyModule_AddIntMacro() compatibility */
+#if !HAVE_DECL_PYMODULE_ADDINTMACRO
+#define PyModule_AddIntMacro(m, c) PyModule_AddIntConstant(m, sss_py_const_p(char, #c), c)
+#endif
+
+/* Convenience macros */
+#define TYPE_READY(module, type, name) do {         \
+    if (PyType_Ready(&type) < 0)                    \
+        return;                                     \
+    Py_INCREF(&type);                               \
+    PyModule_AddObject(module,                      \
+                       discard_const_p(char, name), \
+                       (PyObject *) &type);         \
+} while(0);                                         \
+
+#define SAFE_SET(old, new) do {         \
+    PyObject *__simple_set_tmp = NULL;  \
+    __simple_set_tmp = old;             \
+    Py_INCREF(new);                     \
+    old = new;                          \
+    Py_XDECREF(__simple_set_tmp);       \
+} while(0);
+
+#endif /* __SSS_PYTHON_H__ */
-- 
1.7.5.4

Attachment: signature.asc
Description: OpenPGP digital signature

_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel

Reply via email to