[PATCH] s4-drs: Synchronous Implementation of generated ParentGUID - ToDo List ParentGUID fix

[Fernando Silva]

Hi Tridge!

tridge at samba.org escreveu:
> The changes in construct_parent_guid() looks mostly good. It would be
> a bit neater to move the code that puts a objectGUID into a message
> into a new helper function in dsdb/common/util.c, perhaps called
> dsdb_msg_add_guid().
>   
That's Ok! I'm going to do it!
> The change in operational_search() is not correct. You can see what is
> wrong by trying the following two searches, which should produce the
> same result:
>
>   ldbsearch -H $PREFIX/private/sam.ldb objectclass=user createtimestamp parentguid 
>   ldbsearch -H $PREFIX/private/sam.ldb objectclass=user parentguid createtimestamp
>
> If you try it with your patch, you'll see that the first search
> correctly produces both the createtimestamp and parentguid attributes,
> but the 2nd search only produces the parentguid. The reason is that
> you are putting a NULL value into the search_attrs[] array, which
> terminates the array, so the backend never sees the request for the
> createtimestamp attribute.
Hmm ... that's an interesting issue ... I didn't know it could happen
and I didn't see it when testing ... I will fix it too ...

> For your changes in objectclass.c, please just remove the
> objectclass_rename_callback() function, rather than using #if 0. I
> know that during the tutorial I used an #if 0, but that was just to
> illustrate what to remove, I didn't mean you to patch it like this.
>   
Ok! Sorry for that! Actually I understood your goal when you have done
it on our tutorial. I just saw another piece of code like that into
another commit and I thought it could be helpfull to use it instead
removing that function ... but I was wrong, in my case it's not really
necessary... I will remove it!
> I also suspect you could remove a bit more code from objectclass.c. If
> you look in objectclass_rename() then you'll see this comment:
>
> 	/* note that the results of this search are kept and used to
> 	   update the parentGUID in objectclass_rename_callback() */
>
> perhaps we could remove that search completely? You should see if you
> can, making sure that everything still works (check the code
> carefully, and also run "make test").
>   
I've already tried to remove it ... it seems that ldbrename still work,
but I have to test it more carefully (that's why I got a little afraid
and didn't remove it yet in this patch ...). This bring me a little
doubt: Does ldbrename only changes the object's dn? I only tested it on
ldbrename... Is there another way I should test to ensure it really works?

> You're also getting a warning like this in your code:
>
>  dsdb/samdb/ldb_modules/operational.c:173: warning: passing argument 3 of $B!F(Bldb_msg_add_value$B!G(B from incompatible pointer type
>  lib/ldb/include/ldb.h:1744: note: expected $B!F(Bconst struct ldb_val *$B!G(B but argument is of type $B!F(Bstruct datablob *$B!G(B
>
> if you remember I was a bit puzzled by this when we came across it
> during our tutorial session. I've now found that the cause is that
> operational.c has the wrong #include lines at the top. The
> "includes.h" line should come first, as that defines the override for
> the struct ldb_val to be the same as a struct datablob.
Thanks! I will put the includes in the right order!


Thanks for all comments Tridge! I will work on those issues and I hope
to send another patch soon!


Best regards,

-- 
Fernando J V da Silva
M Sc Computer Science Student
Institute of Computing, State University of Campinas
+55 15 8801-2165

__________________________________________________
Faça ligações para outros computadores com o novo Yahoo! Messenger 
http://br.beta.messenger.yahoo.com/