[SSSD] Re: [PATCH] sdap: improve filtering of multiple results in GC lookups

[Sumit Bose]

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 [e-mail]' 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
> sssd-devel mailing list
> [e-mail]
> https://lists.fedorahosted.org/admin/lists/[e-mail]