[PATCH] s4-drs: Removes stored parentGUID's creation and renaming

[tridge at samba.org]

Hi Fernando,

 > This patch removes the creation and renaming of the stored parentGUID when a
 > object is added or renamed. (needed to solve parentGUID fix issue on ToDo
 > list http://wiki.samba.org/index.php/Samba4_DRS_TODO_List#Parentguid_fix)

I think this looks pretty good, and like the previous patch it is
close to being accepted (as it depends on the other two patches, I'd
have to accept the others first, otherwise parentGUID would break).

The only comments I have are on formatting.

 >  	ret = ldb_build_search_req(&search_req, ldb,
 > -				   ac, parent_dn, LDB_SCOPE_BASE,
 > -				   "(objectClass=*)",
 > -				   attrs, NULL, 
 > -				   ac, get_search_callback,
 > -				   req);
 > +					   ac, parent_dn, LDB_SCOPE_BASE,
 > +					   "(objectClass=*)",
 > +					   attrs, NULL,
 > +					   ac, get_search_callback,
 > +					   req);
 > +

The above section of the patch seems to be just reformatting with no
actual code changes?

It is a good idea to read a patch before you submit it, and tidy it up
to remove anything that should not be there.

Given the above change, I also wonder if perhaps your editor is not
setup to use tabs in the way we use them in the Samba coding style? If
you use emacs or vi then there are instructions on how to setup your
editor in README.Coding. If you use some other editor then please find
out how to set it up to use tabs for C coding, and to display the tabs
as width 8.

 >  	ret = ldb_build_rename_req(&rename_req, ldb, ac,
 > -				   ac->req->op.rename.olddn, fixed_dn,
 > -				   ac->req->controls,
 > -				   ac, objectclass_rename_callback,
 > -				   ac->req);
 > +					   ac->req->op.rename.olddn, fixed_dn,
 > +					   ac->req->controls,
 > +					   ac, oc_op_callback,
 > +					   ac->req);
 > +
 > +

this one does have a real change, but it is hard to spot the change
because of all the whitespace reformatting. The patch should just look
like this:

        ret = ldb_build_rename_req(&rename_req, ldb, ac,
                                   ac->req->op.rename.olddn, fixed_dn,
                                   ac->req->controls,
-                                  ac, objectclass_rename_callback,
+                                  ac, oc_op_callback,
                                   ac->req);

notice that the above makes it much clearer what has actually been
changed? To get this right you'll need to make sure you only change
the line you mean to change, and not reformat the surrounding code.

btw, I haven't actually tested your patches with 'make test' this time
around. I will obviously do that before accepting them into the tree,
but for the moment I am assuming that you are testing them, so I'm
just trying to spot errors by looking at the patches.

Have you thought about adding a test case to
lib/ldb/tests/python/ldap.py for your patches? I would be happy to
accept your patches without a test case, but it would be good if you
could submit a further patch afterwards to add a test case that
demonstrates what you have fixed.

Cheers, Tridge