On Fri, Mar 19, 2010 at 02:15:06PM +0100, Sumit Bose wrote:
On Fri, Mar 19, 2010 at 08:48:49AM -0400, Simo Sorce wrote:
> On Fri, 19 Mar 2010 12:00:47 +0100
> Sumit Bose <sbose(a)redhat.com> wrote:
>
> > On Thu, Mar 18, 2010 at 05:51:13PM -0400, Simo Sorce wrote:
> > >
> > > Some time ago I added code to fetch the rootdse on connection, but
> > > didn't publicize it too much.
> > >
> > > Attached find 2 patches.
> > >
> > > 1) Rework the way we store data fetched from the rootdse so the it
> > > is more useful and is actually attached to the ldap handle.
> >
> > I'm in general fine with this one, although I haven't tested it
> > throughly yet.
> > >
> > > 2) Check controls are supported before using them.
> >
> > I have two comments here:
> >
> > - sdap_is_control_supported() should be integrated in
> > sss_ldap_control_create() or a new call should be created which
> > calls sdap_is_control_supported() and then sss_ldap_control_create().
> > - the debug message 'Server does not support the Password Policy
> > control' should have a much lower level than 7. Because if the
> > control is missing the user might not see the expected
> > behaviour/performance.
>
> Sumit,
> do you want to take over the second patch ?
> I don't have a way to thoroughly test it anyway and I am a bit short on
> time.
sure
Hi,
not even 4 months later here are the revised versions of these patches.
I had it add two more changes to the first patch to make it build on
master because of the change to sdap_cli_connect_recv().
In the second patch if have created a new call sdap_control_create()
which checks if the control is supported with
sdap_is_control_supported() and calls sss_ldap_control_create() on
success. I think this approach is more clean, because it leaves
sss_ldap_control_create() as a pure replacement function.
bye,
Sumit