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

Fernando Silva fernandojvsilva at yahoo.com.br
Fri Nov 13 19:30:54 MST 2009

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 ‘ldb_msg_add_value’ from incompatible pointer type
>  lib/ldb/include/ldb.h:1744: note: expected ‘const struct ldb_val *’ but argument is of type ‘struct datablob *’
> 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 

More information about the samba-technical mailing list