[SSSD] [PATCH] Fix python HBAC bindings for python <= 2.4

Alexander Bokovoy abokovoy at redhat.com
Wed Jul 13 09:09:38 UTC 2011


Hi,

On 13.07.2011 11:45, Jakub Hrozek wrote:
>> I would NACK the patch. Few issues:
>>
>> 1. char *str_concat_sequence(PyObject *seq, char *delim) use is always
>> with constant delim, it would make sense to change the signature to
>> follow it.
> Fixed in patch #1
ACK.

>> 2. discard_const_p() for Python 2.4 is a bit too wide use as it weakens
>> 2.6 (and 3.0) API usage. What if you make a simple wrapper on top of
>> discard_const_p() that is ident to it for 2.4 but returns the actual
>> value without type conversion for other cases.
>>
>> Something like this:
>> #if PYTHON_2_4
>> #define sss_py_const_p(type, value) discard_const_p(type, (value))
>> #else
>> #define sss_py_const_p(type, value) (value)
>> #endif
> Fixed in patch #2
ACK.

>> 3. hbac_rule_set_enabled() currently is too strict, it accepts only
>> Python's boolean. In FreeIPA LDAP backend returned values are strings so
>> we'll have to build up a wrapper on top of pyhbac. What about be more
>> flexible and accepting what FreeIPA's Bool parameter type accepts:
>>    ('truths', frozenset, frozenset([1, u'1', u'true', u'TRUE'])),
>>    ('falsehoods', frozenset, frozenset([0, u'0', u'false', u'FALSE']))
>>
>
> Fixed in patch #1
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!
-- 
/ Alexander Bokovoy




More information about the sssd-devel mailing list