[SSSD] [PATCHES] Initial refactoring for #1747

Jakub Hrozek jhrozek at redhat.com
Thu Jan 10 19:34:32 UTC 2013


On Thu, Jan 10, 2013 at 08:29:36PM +0100, Jakub Hrozek wrote:
> On Thu, Jan 10, 2013 at 12:39:53PM -0500, Simo Sorce wrote:
> > On Tue, 2013-01-08 at 23:15 -0500, Simo Sorce wrote:
> > > Attached find a somewhat big patchset composed of 59 patches.
> > > These patches start to address #1747 by changing a fundamental issue in
> > > the current code where the sysdb_ctx structure includes a reference to a
> > > sss_domain_info one.
> > > 
> > > The issue is that we now have multiple named domain in a single sysdb
> > > database so a univocal sysdb->domain relationship doesn't represent
> > > reality anymore.
> > > 
> > > So far we have coped by somewhat faking up sysdb contexts for domains
> > > but this is all backwards and makes it hard to predict if code is
> > > behaving correctly.
> > > 
> > > Although this patchset is huge the bulk is just a series of painstaking
> > > patches to change a ton of interfaces to take a sss_domain_info
> > > structure in input so we can avoid ambiguity about which domain the
> > > sysdb is asked to deal with.
> > > 
> > > As mentioned this is just an 'initial' patchset, there are still other
> > > aspects to handle as part o #1747 but this patchset is self-contained
> > > and big enough it would only make it harder to get in if we waited for
> > > more changes to pile on top.
> > > 
> > > The patchset is attached in tar.gz format (sorry but unpacked is too
> > > big, ~650K)
> > > 
> > > I will keep rebasing this[0] branch on top of master with this same
> > > patchset, and I recommend for your sanity to pull from there rather than
> > > try to apply 59 patches for patchfiles. The reason is that this patchset
> > > touches a lot of code and any new commit in master is almost certainly
> > > going to require a rebase to this patchset.
> > > 
> > > Keep in mind that the general principle in building this patchset was to
> > > try to have the least impact on the code, not necessarily to beautify
> > > the code I was going to touch, this in order to make it very clear what
> > > is changed and ckeep churn to a bare minimum, so I haven't changed
> > > things like DEBUG levels to avoid any noise and additional great pain in
> > > the inevitable rebases that are to come.
> > > 
> > > I did test the code on my install with multiple trusts, and it seem to
> > > be working correctly with normal and subdomain users.
> > > 
> > > Happy reviewing :)
> > > 
> > > Simo.
> > > 
> > > [0]
> > > http://fedorapeople.org/cgit/simo/public_git/sssd.git/log/?h=sysdb_refactor
> > > 
> > > Note: The git tree has actually 63 patchse on top of master, the first 4
> > > are pre-requisite patches I already sent to the list for review
> > > separately.
> > 
> > FYI: I rebased my public git tree against the current master as the
> > latest patches that went in caused conflicts.
> 
> That's fine, I started reviewing the patches from your git tree. I still
> think it was a good idea to send the patch to the ML as the archive will
> keep them forever :-)
> 
> I'll make comments as replies to this thread.

Probably needs another rebase after yesterday's AD patches:
  CC       src/providers/ldap/sdap_async_services.lo
src/providers/ldap/sdap_async_groups.c: In function 'sdap_get_members_with_primary_gid':
src/providers/ldap/sdap_async_groups.c:117:30: warning: passing argument 3 of 'sysdb_search_users' from incompatible pointer type [enabled by default]
In file included from src/providers/ldap/sdap_async_groups.c:25:0:
./src/db/sysdb.h:746:5: note: expected 'struct sss_domain_info *' but argument is of type 'char *'
src/providers/ldap/sdap_async_groups.c:117:30: warning: passing argument 4 of 'sysdb_search_users' from incompatible pointer type [enabled by default]
In file included from src/providers/ldap/sdap_async_groups.c:25:0:
./src/db/sysdb.h:746:5: note: expected 'const char *' but argument is of type 'const char **'
src/providers/ldap/sdap_async_groups.c:117:30: warning: passing argument 5 of 'sysdb_search_users' from incompatible pointer type [enabled by default]
In file included from src/providers/ldap/sdap_async_groups.c:25:0:
./src/db/sysdb.h:746:5: note: expected 'const char **' but argument is of type 'size_t *'
src/providers/ldap/sdap_async_groups.c:117:30: warning: passing argument 6 of 'sysdb_search_users' from incompatible pointer type [enabled by default]
In file included from src/providers/ldap/sdap_async_groups.c:25:0:
./src/db/sysdb.h:746:5: note: expected 'size_t *' but argument is of type 'struct ldb_message ***'
src/providers/ldap/sdap_async_groups.c:117:30: error: too few arguments to function 'sysdb_search_users'
In file included from src/providers/ldap/sdap_async_groups.c:25:0:
./src/db/sysdb.h:746:5: note: declared here

To spare you from further headaches, I think we need to review this patchset
before doing any more changes.



More information about the sssd-devel mailing list