[SCM] Samba Shared Repository - branch master updated - 3d0a3c1078d936a52ce82720ddc09b4037653655

[simo]

On Tue, 2008-11-18 at 13:08 +1100, Andrew Bartlett wrote:
> On Mon, 2008-11-17 at 07:42 -0500, simo wrote:

> > > commit 16a3a2da78b1f2d5a1077e382a26466944f6c59e
> > > Author: Andrew Bartlett <abartlet at samba.org>
> > > Date:   Thu Nov 13 14:07:29 2008 +1100
> > > 
> > >     Remove timeout event once we are calling the callback.
> > >     
> > >     (Even if the callback takes some time, this isn't a ldb_tdb timeout
> > >     any more)
> > >     
> > >     Andrew Bartlett
> > 
> > I think this is wrong Andrew.
> > The callback is fully part of the call and the timeout should apply.
> > You may rise the timeout time if you think that normally we should give
> > more time. But certainly not remove the timeout imo.
> 
> So who should remove the timeout?
> 
> With this in place, the timeout hangs around until talloc happens to
> clean it up.  This means that an extra event gets propagated up the
> callbacks, after the 'done' is processed, and quite possibly to use no
> longer valid context pointers.  This is particularly important for an
> operation that will take on other operations later. 

Right,
I am sorry I misread the initial patch.
I think the way you did it is ok.

> > > commit 109719de030cb2432bea991077b12b4cf937c108
> > > Author: Andrew Bartlett <abartlet at samba.org>
> > > Date:   Fri Nov 14 16:34:59 2008 +1100
> > > 
> > >     Remove restrictions on number of DN components in LDAP server
> > >     
> > >     There is no reason for these restrictions to be in the LDAP server -
> > >     they belong in the LDB layer.  When accepting 'extended' or
> > >     'alternate' DNs we can't tell anyway.
> > 
> > No, they certainly do not belong in LDB, as LDB does not have the same
> > restrictions as the LDAP server. I would like you to put them back.
> 
> I strongly disagree, and in any case it is not possible once we accept
> extended DN searches/modifies/deletes.  We already had to add hacks to
> allow a modify of "" (required for schemaUpdateNow on the rootDSE).
> 
> At the time (Subject: 	Re: How to process schemaUpdateNow ldap request
> Date: Tue, 01 Jul 2008 08:45:53 -0400) you said:
> 
> > Andrew Bartlett wrote:
> > > If you look at ldap_server/ldap_backend.c, the macro VALID_DN_SYNTAX
> > > takes two argument, the first being the DN, and the second is the
> > number
> > > of components it must have.  Set that to 0 and you should be right.
> > > 
> > > I don't see why this layer should be trying to determine if a DN is
> > > valid (ldb can do that very well itself).  This looks like Simo's
> > code,
> > > according to 'git blame', so I'll flip-pass this question to him...
> > 
> > I think we added it before ldb was able to validate, then kept it for
> > performance reasons, it make no sense to process the entry if it is
> > going to be rejected.
> > However a null DN should not be refuse I guess, feel free to patch the
> > code to let that DN be considered valid (as it is).
> > 
> > Simo.
> 
> Given we must accept a NULL dn in these cases, I think we should accept
> any syntactic valid DN, and let LDB figure out what is or is not a
> possible DN, based on the actual content of the database. 

Uhmmm, I guess you are right, seen in that context it seem we cannot do
the checks, nor in LDAP, nor in LDB until we hit the backend.

I wish you referenced the above in the commit message, it would have
been more clear.
 
> > >     Always validate a DN when constructing from a string in python
> > 
> > This may make the python bindings much slower ... but perhaps that's ok
> > for the bindings.
> 
> I think it is better to check there than later where we cannot catch the
> error as easily.

Yes, I don't think the python bindings are too performance sensitive,
except perhaps at setup time, but even if that is a bit slower it
shouldn't be a big deal.

Simo.

-- 
Simo Sorce
Samba Team GPL Compliance Officer <simo at samba.org>
Senior Software Engineer at Red Hat Inc. <simo at redhat.com>