LDB patches the return

[Matthieu Patou]

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.
>
> 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.
>
> 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).

Matthieu

-- 
Matthieu Patou
Samba Team
http://samba.org