Bug 7485 - Feature request: Add "ldb_msg_element_compare" functions which work with syntax comparison (rather than "memcmp")
Summary: Feature request: Add "ldb_msg_element_compare" functions which work with synt...
Status: ASSIGNED
Alias: None
Product: Samba 4.0
Classification: Unclassified
Component: AD: LDB/DSDB/SAMDB (show other bugs)
Version: unspecified
Hardware: Other Linux
: P3 enhancement (vote)
Target Milestone: ---
Assignee: Matthieu Patou
QA Contact: samba4-qa@samba.org
URL:
Keywords:
Depends on:
Blocks: 8929
  Show dependency treegraph
 
Reported: 2010-06-01 16:03 UTC by Matthieu Patou
Modified: 2018-09-07 02:42 UTC (History)
5 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Matthieu Patou 2010-06-01 16:03:23 UTC
While running upgradeprovision, msg_diff found a modification on the member attribute. 

Here is the content of the old and the new attribute

in old: CN=Read-Only Domain Controllers,CN=Users,DC=home,DC=matws,DC=net
in old: CN=Group Policy Creator Owners,CN=Users,DC=home,DC=matws,DC=net
in old: CN=Domain Admins,CN=Users,DC=home,DC=matws,DC=net
in old: CN=Cert Publishers,CN=Users,DC=home,DC=matws,DC=net
in old: CN=Enterprise Admins,CN=Users,DC=home,DC=matws,DC=net
in old: CN=Schema Admins,CN=Users,DC=home,DC=matws,DC=net
in old: CN=Domain Controllers,CN=Users,DC=home,DC=matws,DC=net
in old: CN=krbtgt,CN=Users,DC=home,DC=matws,DC=net
in new: CN=Read-only Domain Controllers,CN=Users,DC=home,DC=matws,DC=net
in new: CN=Group Policy Creator Owners,CN=Users,DC=home,DC=matws,DC=net
in new: CN=Domain Admins,CN=Users,DC=home,DC=matws,DC=net
in new: CN=Cert Publishers,CN=Users,DC=home,DC=matws,DC=net
in new: CN=Enterprise Admins,CN=Users,DC=home,DC=matws,DC=net
in new: CN=Schema Admins,CN=Users,DC=home,DC=matws,DC=net
in new: CN=Domain Controllers,CN=Users,DC=home,DC=matws,DC=net
in new: CN=krbtgt,CN=Users,DC=home,DC=matws,DC=net

it's the same just the case for Read-only Domain Controllers change
Comment 1 Matthias Dieter Wallnöfer 2010-06-02 02:37:09 UTC
ekacnet - well if you look at "lib/ldb/common/ldb_msg.c" then you notice that it is exactly like that (also written in the comments). It's by design since the comparison of the attribute values is done bytewise (binary).
I would suggest to not change the LDB interface (perhaps write some wrapper directly in the "upgradeprovision" script).
Comment 2 Matthieu Patou 2010-06-02 03:46:21 UTC
Of course I can cope with it in upgradeprovision (it's already the case btw).

The question that has to be solved is how ldb_msg_diff should make the comparison between 2 attribute, either bitwise or by using attribute comparator if available.

Maybe we should keep the default behavior and have the other way as an option so that we didn't break what is already making this assumption but we allow a more logical behavior. 
Comment 3 Matthias Dieter Wallnöfer 2010-06-02 05:35:40 UTC
I would add the other option as a new function to don't break compatibility (not only a new parameter).
But if you would like to have this changed then I would discuss it on the mailing list; especially with Simo which is the library maintainer.
Comment 4 Matthias Dieter Wallnöfer 2010-06-07 15:47:09 UTC
Well, I close this with "WONTFIX" here and we should start some discussion on the mailing list (or write yourself a wrapper in "upgradeprovision").
Comment 5 Matthieu Patou 2010-06-07 16:01:08 UTC
Why such hurry to close this bug. 
From my point of view the bug is there: ldb_msg_diff do not use the compare function of each element to decide whether compared element are equal or not.

After I agree that changing the current behavior is not the better thing.

I reopen this bug until we have the time to discuss this, if the bug was 6 months old I could understand your will to close it. But right now, no sorry it's not the good way to my mind. As you're somehow denying the existance of the problem or reducing it just to upgradeprovision and it's clearly not a pb just for upgradeprovision.
Comment 6 Matthias Dieter Wallnöfer 2010-06-08 09:58:16 UTC
Okay, agreed - we will track this issue here.

Idra, would you be fine for a change in the "ldb_msg_element_compare" function or in the "ldb_val_equal_exact" function (ldb_msg.c) in order to allow real value comparisons through the attribute syntax functions?
Or should we better introduce an additional function which cares about this?
Comment 7 Matthias Dieter Wallnöfer 2010-06-10 15:40:12 UTC
idra, could you please express your opinion about a "ldb_msg_element_compare" change?
Comment 8 Simo Sorce 2010-06-10 15:58:52 UTC
What you propose is a change in behaviour.
Before changing it, it would need extensive tests to make sure everything continues to work, especially with standalone ldb.
Comment 9 Matthieu Patou 2010-06-10 17:15:31 UTC
(In reply to comment #8)
> What you propose is a change in behaviour.
> Before changing it, it would need extensive tests to make sure everything
> continues to work, especially with standalone ldb.
> 

I would rather suggest that we agree that yes it's not the best behavior, like that but that it's maybe safer to create a second ldb_msg_diff that do use correct compare function.
Comment 10 Andrew Bartlett 2010-06-10 17:34:29 UTC
Indeed, the difficult issue here is to determine if:
 - The caller wanted to know if there are case differences (so it could fix them)
or
 - The caller wanted to know if the messages had the same semantic meaning

These are two very different issues, and need different functions.  We should not change the LDB API at this fundamental level, so a 'same semantic meaning' function will need to have a different name. 
Comment 11 Simo Sorce 2010-06-10 17:40:05 UTC
I tend to agree with Andrew here.
Comment 12 Matthias Dieter Wallnöfer 2010-09-11 12:38:39 UTC
Ekacnet, should we still implement such comparison calls (I mean in the core LDB)?
Comment 13 Matthias Dieter Wallnöfer 2010-10-01 08:39:45 UTC
Ekacnet, do you still want to have this bug open?
Comment 14 Matthieu Patou 2010-10-01 11:48:54 UTC
yes
Comment 15 Matthias Dieter Wallnöfer 2010-11-03 16:44:17 UTC
Ekacnet, I find the best would be if you write such a patch or at least a proposal - so that we are able to see how you would like to have this interacting with the existing code.
I mean for example would you like to have a function comparison pointer on the "ldb_msg_element_compare" call in order to distinguish between the comparison methods? Or would you be satisfied with a bool flag which switches the comparison mode?
Comment 16 Kamen Mazdrashki 2010-11-03 17:34:28 UTC
(In reply to comment #15)
I don't think we should change current implementation, but rather implement another one in DSDB layer. 

Comment 17 Matthias Dieter Wallnöfer 2011-03-07 10:05:57 UTC
Ekacnet, I'm assigning this up to you.

This will only be implemented if you provide the patches I think.