important: Wrong logic (to my mind) in schema_set.c

Matthieu Patou mat+Informatique.Samba at matws.net
Sun Nov 29 07:02:32 MST 2009


Dear all, I've been trying to upgrade a very old provision (alpha3) I've 
been hit by errors like this:

_ldb.LdbError: (80, 'schema_load_init: dsdb_set_schema() failed: 32:No 
such object')
After a careful investigation I am 99% sure that those two changes 
(introduced in changeset 6b05a907) are wrong

@@ -135,7 +135,7 @@ static int dsdb_schema_set_attributes(struct 
ldb_context *ldb, struct dsdb_schem

                 mod_msg = ldb_msg_diff(ldb, res->msgs[0], msg);
                 if (mod_msg->num_elements > 0) {
-                       ret = ldb_modify(ldb, mod_msg);
+                       ret = samdb_replace(ldb, mem_ctx, mod_msg);
                 }
         }

@@ -163,7 +163,7 @@ static int dsdb_schema_set_attributes(struct 
ldb_context *ldb, struct dsdb_schem

                 mod_msg = ldb_msg_diff(ldb, res_idx->msgs[0], msg_idx);
                 if (mod_msg->num_elements > 0) {
-                       ret = ldb_modify(ldb, mod_msg);
+                       ret = samdb_replace(ldb, mem_ctx, mod_msg);
                 }

Because the different elements with different flags (add,replace,delete) 
and using samdb_replace will override this flag by using just replace. 
Obviously if the ldb_diff was telling you that you should add the 
attribute 'foo' trying to make ldb modify the 'foo' attribute will not 
be very good.

Also I'm quite puzzled by the use ldb_msg_diff and the order of 
parameters. Because res->msgs[0] is an message element obtained from a 
search in the LDB database represented by the ldb object.
So to my mind by doing
mod_msg = ldb_msg_diff(ldb, res->msgs[0], msg);
we are comparing res->msgs[0] and msg and using res->msgs[0] as a 
reference, so there is no point trying to apply changes in the ldb 
object as the attributes have already the value contained in the 
different messageElement of the mod_msg object.

So I think parameters should be swapped it makes just sens from the 
point of view of the comparison and the modification of the ldb but for 
the overall logic I'm much more reserved.

Can someone have a closer look on this ?

Matthieu.


More information about the samba-technical mailing list