[SSSD] [PATCH] refactor nested group processing

Lukas Slebodnik lslebodn at redhat.com
Thu Mar 28 15:33:51 UTC 2013


On (27/03/13 11:03), Lukas Slebodnik wrote:
>On (25/03/13 14:54), Jakub Hrozek wrote:
>>On Mon, Mar 25, 2013 at 02:48:44PM +0100, Pavel Březina wrote:
>>> On 03/22/2013 10:59 PM, Lukas Slebodnik wrote:
>>> >On (21/03/13 16:10), Pavel Březina wrote:
>>> >>On 03/21/2013 03:43 PM, Lukas Slebodnik wrote:
>>> >>>On (22/02/13 13:46), Pavel Brezina wrote:
>>> >>>>----- Original Message -----
>>> >>>>>From: "Pavel Březina" <pbrezina at redhat.com>
>>> >>>>>To: sssd-devel at lists.fedorahosted.org
>>> >>>>>Sent: Thursday, February 21, 2013 11:43:26 AM
>>> >>>>>Subject: [SSSD] [PATCH] refactor nested group processing
>>> >>>>>
>>> >>>>>https://fedorahosted.org/sssd/ticket/1784
>>> >>>>
>>> >>>>I should have probably add that this is the final version. I will attach unit test once I get familiar with cmocka, but it does not really prevent reviewing the patch.
>>> >>>
>>> >>>I would like to test your patches, but the second patch no longer applies
>>> >>>on the current master. Could you resend rebased patches?
>>> >>
>>> >>Thanks. Rebased patches are attached. I did not test it again though,
>>> >>but it compiles fine and the diff looks all right. It may be
>>> >>interesting to determine what patch broke it.
>>> >>
>>> >
>>> >I had problems with you patches.
>>> >The simplest cycle schema:
>>> >    ________
>>> >    |  CG1 |------------>sssduser1
>>> >    |______|<---|
>>> >       |        |
>>> >       |        |
>>> >       |     ________
>>> >       |---->|  CG2 |--->sssduser2
>>> >             |______|
>>> >
>>> >sssd client configured with ipa-client-install and two additional options
>>> >entry_cache_timeout = 5
>>> >memcache_timeout = 1
>>> >
>>> >How to reproduce.
>>> >1. remove cache(also logs, but it is not important)
>>> >2. service sssd start
>>> >3. run commands
>>> >
>>> >sh-4.2$ getent group cyclegroup1
>>> >cyclegroup1:*:30007:sssduser1,sssduser2
>>> >sh-4.2$ getent group cyclegroup2
>>> >cyclegroup2:*:30008:sssduser2
>>> >  #after second (memcache_timeout)
>>> >sh-4.2$ getent group cyclegroup2
>>> >cyclegroup2:*:30008:sssduser2,sssduser1
>>> >
>>> >-----------------------------
>>> >
>>> >Also sometimes with the same groups scheme one user appeared two times,
>>> >but I could not exactly reproduce steps.
>>> 
>>> The problem is even in master. Seems to be another ghost users issue.
>>> 
>>
>>I agree, the EEXIST error is coming from memberof plugin:
>>https://fedorahosted.org/sssd/ticket/1846
>>
>>> >
>>> >If you could not reproduce problem, I could send you off the list
>>> >logs, cache, any configurations, rpms or even provide access to IPA server.
>>> >
>>> >And last notice, commit 4709ff46db0dbe073aef061b796d2fd7adeaf18f
>>> >with title "LDAP: If deref search fails, try again without deref"
>>> >cause little problem with applying patches.
>>> >
>>> >LS
>>> 
>>> I amended my patches to keep the functionality introduced in this
>>> commit. I.e. if deref fails with ENOTSUP, we will try again without
>>> dereference.
>>> 
>
>I tested functionality of your patches and except bug from ticket
>(https://fedorahosted.org/sssd/ticket/1846) everything work like expected.
>Tested cases:
>    --nonexisting group
>    --empty group
>    --user in group
>    --nested groups
>    --user in more nested group(no duplication)
>    --cycle
>    --deref
>    --nonderef
>
>I have to say, that I learnd a lot of new things.
>
>(mini Ack) -> So someone else could look to code.
>I am not yet expert in tevent coding style
>
>LS

You introduced compile time warning. I did not noticed it first time,
because clang do not consider this to be a warning.

src/providers/ldap/sdap_async_nested_groups.c: In function 'sdap_nested_group_single_done':
src/providers/ldap/sdap_async_nested_groups.c:1143:41: warning: 'type'
may be used uninitialized in this function [-Wmaybe-uninitialized]
src/providers/ldap/sdap_async_nested_groups.c:1128:36: note: 'type' was declared here

Second similar warning is already in master.

src/providers/ldap/sdap_async_connection.c: In function 'sdap_cli_kinit_done':
src/providers/ldap/sdap_async_connection.c:1558:28: warning:
'expire_time' may be used uninitialized in this function [-Wmaybe-uninitialized]
src/providers/ldap/sdap_async_connection.c:1545:12: note: 'expire_time' was declared here

Both warning is the same type, like fixed in commit
http://git.fedorahosted.org/cgit/sssd.git/commit/?id=2d75dbbc6549acb1f749b5366b492ae087fdc0d8

LS



More information about the sssd-devel mailing list