On Thu, Feb 25, 2016 at 01:37:27PM +0100, Sumit Bose wrote: > On Thu, Feb 25, 2016 at 12:50:55PM +0100, Jakub Hrozek wrote: > > On Tue, Feb 23, 2016 at 12:53:25PM +0100, Sumit Bose wrote: > > > Hi, > > > > > > this patch fixes and issue during initgroups in AD forests. Please see > > > the commit message for details. > > > > > > To reproduce this you can create a new user outside of CN=Users on the > > > forest root. The new user can be created in an existing container or in > > > a new OU container. Most important is that it is not a child of > > > CN=Users. In a child domain (it must be a child, domains with a > > > different base won't trigger the issue) create a user with the same > > > name. With this setup 'id [email protected]' will not return the complete > > > list of group the user is a member of and the patch should fix this. > > > > > > bye, > > > Sumit > > > > > > > Hi, > > > > the patch works fine, tested with a user named the same in the same OU > > in different domains. Coverity found some warnings: > > > > Error: COMPILER_WARNING: > > sssd-1.13.90/src/providers/ldap/sdap_async_initgroups.c:2835:12: warning: > > unused variable 'dn_len' [-Wunused-variable] > > # size_t dn_len; > > # ^ > > # 2833| const char *cname; > > # 2834| bool in_transaction = false; > > # 2835|-> size_t dn_len; > > # 2836| size_t c = 0; > > # 2837| > > > > Error: COMPILER_WARNING: > > sssd-1.13.90/src/providers/ldap/sdap_async_initgroups.c: scope_hint: In > > function 'sdap_get_initgr_user' > > sssd-1.13.90/src/providers/ldap/sdap_async_initgroups.c:2836:12: warning: > > unused variable 'c' [-Wunused-variable] > > # size_t c = 0; > > # ^ > > # 2834| bool in_transaction = false; > > # 2835| size_t dn_len; > > # 2836|-> size_t c = 0; > > # 2837| > > # 2838| DEBUG(SSSDBG_TRACE_ALL, "Receiving info for the user\n"); > > > > Error: COMPILER_WARNING: > > sssd-1.13.90/src/db/sysdb_subdomains.c:23: included_from: Included from > > here. > > sssd-1.13.90/src/db/sysdb_subdomains.c: scope_hint: In function > > 'try_to_find_expected_dn' > > sssd-1.13.90/src/util/util.h:144:9: warning: 'result_dn_str' may be used > > uninitialized in this function [-Wmaybe-uninitialized] > > # sss_debug_fn(__FILE__, __LINE__, __FUNCTION__, \ > > # ^ > > sssd-1.13.90/src/db/sysdb_subdomains.c:1075:17: note: 'result_dn_str' was > > declared here > > # const char *result_dn_str; > > # ^ > > # 142| int __debug_macro_level = level; \ > > # 143| if (DEBUG_IS_SET(__debug_macro_level)) { \ > > # 144|-> sss_debug_fn(__FILE__, __LINE__, __FUNCTION__, \ > > # 145| __debug_macro_level, \ > > # 146| format, ##__VA_ARGS__); \ > > > > Additionally, I wonder if we should namespace the function? > > Thank you for the review, new version attached. > > bye, > Sumit
ACK. There were some downstream tests failing, but the same tests kept failing even with a vanilla RHEL-7.2 package, so I think it's a fluke in the test and not a regression. _______________________________________________ sssd-devel mailing list [email protected] https://lists.fedorahosted.org/admin/lists/[email protected]
