LDB patches the return

[Andrew Bartlett]

On Sun, 2013-05-19 at 10:34 -0700, Matthieu Patou wrote:
> On 05/18/2013 04:20 AM, Andrew Bartlett wrote:
> > On Sat, 2013-05-18 at 03:48 -0700, Matthieu Patou wrote:
> >> On 05/17/2013 09:26 PM, Andrew Bartlett wrote:
> >>> On Fri, 2013-05-17 at 10:09 -0700, Matthieu Patou wrote:
> >>>> Hello all,
> >>>>
> >>>> This is the return of my patches for LDB, here is a first set of
> >>>> patches, per se they are not useful it's just the foundation for the
> >>>> other patches.
> >>>>
> >>>> Having them already reviewed will ease the review process for the core ones.
> >>>>
> >>>> b0bb3d6 ldb: Make ldb_match_scope and ldb_match_message available to
> >>>> other ldb parts
> >>>> 9850a73 ldb: Add ldb_dn_set_casefolded so that a DN can marked as
> >>>> already casefolded
> >>>> 986712d ldb: introduce ldb_unpack_data_withlist to unpack partial list
> >>>> of attributes
> >>>> 55794d1 ldb-tdb: split ldb_match_msg_error by 2 seprate calls to
> >>>> ldb_match_scope + ldb_match_message
> >>>> 3cf098d ldb: Upgrade to version 1.1.16 due to newly added functions
> >>>>
> >>>> They are in my wait_reviews branch at
> >>>> http://git.samba.org/mat/?p=mat/samba.git;a=shortlog;h=refs/heads/wait_reviews
> >>>>
> >>>> Please review but do not push
> >>> It would be really good to have a unit test of the unpack code, that
> >>> unpacked a known blob and showed that we got either the full list or
> >>> short list as appropriate.
> >>>
> >> Any other remarks on those patches ?
> > Only that we now have:
> >
> > ldb_match_msg(), ldb_match_msg_error() and ldb_match_message()
> True unfortunately ldb_match_msg should be called match_msg_and_scope().
> Maybe I can add a task so that at the major ldb change we get rid of the 
> redundancy.

I should have also specified a possible part solution:  Can you as well
as making it un-static, also give it a clearer name?  Even
ldb_match_message_tree() would perhaps indicate that this one doesn't
match on the base.

Or at least put some clear documentation in the ldb_private.h

> >
> > Also, now that we check the DN before we unpack, do we validate that the
> > DN in the message matched the one we expected (from the key)?
> >
> > (This strange, buggy condition would have just meant that record getting
> > discarded, but now the buggy record could be thrown further up the
> > stack).
> No I assumed that the key and the dn are in sync, I can have a look but 
> we will have to case fold in order to compare. I'm a in mixed feeling 
> here as it would be a good idea even though the key should be always in 
> sync with the DN stored in the record (unless you are doing some dirty 
> bad thing with the database) and well as most of the time (all the time 
> ?) it's in sync I feel like adding a costly check for nothing.

I guess so.  It's a pity dbcheck isn't able to work at this level and we
have no other tool that could check this.  

> > It also crossed my mind that this is still quite late in the process for
> > these DNs, they have already had to be processed via the index
> > intersection code.  But I guess they are not a handy ldb_dn until this
> > point, and it would be a more major restructure.
> I'm not sure I understand your point, well index entries are treated as 
> an ldb_dn because imho they are a ldb_dn and also because we use it for 
> a fairly short amount of time (just during the indexing part).

I was trying to suggest that a DN that doesn't match the scope could be
prevented from ever entering the index candidate DN list. 

Andrew Bartlett

-- 
Andrew Bartlett                                http://samba.org/~abartlet/
Authentication Developer, Samba Team           http://samba.org