[SSSD] Ticket #1376 -- Add support for suppressing group members

Stephen Gallagher sgallagh at redhat.com
Wed Nov 14 21:41:38 UTC 2012


On Wed 14 Nov 2012 02:28:07 PM EST, Simo Sorce wrote:
> On Wed, 2012-11-14 at 10:59 -0800, Paul B. Henson wrote:
>> On Wed, Nov 14, 2012 at 09:04:24AM -0500, Simo Sorce wrote:
>>
>>> Ok, put down this way it tips my opinion toward the currently proposed
>>> patch.
>>
>> Cool. Is there anything else I need to do to move this forward?
>
> No we just need someone to test and ack your patch.
>
> Simo.
>


First of all, thank you very much for the contribution. It's a great 
start. I've got a few bits and pieces for you to clean up before we 
commit it upstream.

Minor: Please use the new SSSDBG macros in confdb_get_domain_internal(). 
You don't need to update the existing code, but all new code should use 
the macros. See util.h for a listing of them.

Build issue:
../src/providers/ldap/ldap_id.c: In function 'groups_get_send':
../src/providers/ldap/ldap_id.c:448:32: error: passing argument 4 of 
'build_attrs_from_map' from incompatible pointer type [-Werror]
In file included from ../src/providers/ldap/ldap_common.h:26:0,its o
                  from ../src/providers/ldap/ldap_id.c:32:
../src/providers/ldap/sdap.h:481:5: note: expected 'const char **' but 
argument is of type 'char **'
cc1: all warnings being treated as errors

I recommend building with:
make CFLAGS+="-ggdb3 -O0 -Wall -Wextra -Wno-unused-parameter 
-Wno-sign-compare -Wformat-security -Werror"


Test issue:
.....F...F...
======================================================================
FAIL: testListOptions (__main__.SSSDConfigTestSSSDDomain)
----------------------------------------------------------------------
Traceback (most recent call last):
   File "./../src/config/SSSDConfigTest.py", line 539, in testListOptions
     option)
AssertionError: Option [ignore_group_members] unexpectedly found

======================================================================
FAIL: testRemoveProvider (__main__.SSSDConfigTestSSSDDomain)
----------------------------------------------------------------------
Traceback (most recent call last):
   File "./../src/config/SSSDConfigTest.py", line 879, in testRemoveProvider
     option)
AssertionError: Option [ignore_group_members] unexpectedly found

----------------------------------------------------------------------
Ran 13 tests in 0.094s

FAILED (failures=2)


You need to add 'ignore_group_members' to the tuple of expected values 
in the test in SSSDConfigTest.py



Style issue:
      /* TODO: handle attrs_type */
      ret = build_attrs_from_map(state, ctx->opts->group_map, 
SDAP_OPTS_GROUP,
-                               NULL, &state->attrs, NULL);
+                               state->domain->ignore_group_members ?
+                               member_filter : NULL, &state->attrs, NULL);

Please indent the rest of the ?: structure so that it's clear that 
member_filter : NULL is part of the above. &state->attrs, NULL should be 
on the next line. It'll read easier that way.


In sdap_get_groups_done(), wouldn't it just be easier to use 
!state->dom->ignore_group_members with a comment as to what that means? 
The if-then-else construct is a bit confusing there.

nsssrv_cmd.c has added a tab at the beginning of the line
if (!dom->ignore_group_members) {
SSSD is standardized on four-space indentation. (And my editor lights it 
up like neon when tabs are added)


Other than that, the code looks good. I haven't had a chance to test it 
yet, but I'll try to get to that in the next two days.



More information about the sssd-devel mailing list