change to acl_read module for supporting dirsync module

[Matthieu Patou]

On 09/03/2011 12:08, Matthias Dieter Wallnöfer wrote:
> ekacnet,
>
> I've also looked about this patch - two comments:
>
>    * Is there a real reason that the module has to be located below the
>      ACL one? I'm raising this thought since if there is none we could
>      save us an additional control and simply put it higher. We really
>      need to start avoiding controls where not strictly necessary.
>      Another way could be to use the new "trusted/untrusted" mechanism
>      for getting "replPropertyMetaData".
Well to my mind the dirsync module (which implements the dirsync 
control) is higher than the read_acl module the control is set in the 
search function and has to be used in the acl_callback function.
That's why you need the control to tell read_acl not to return nothing 
because replpropertymetadata (or any other attribute) is inaccessible.

I also tried to put it bellow so that we could avoid this control but I 
had more problems as some attributes were removed by the dirsync module 
as they were not modified since last dirsync request. This is especially 
true with the ntsecuritydescriptor as it can be removed because it has 
not been modified since last request and in this case the acl_search 
module will fail as it didn't have the SD. We could of course return 
always the SD but then the acl_read has to remove it in case of dirsync 
control and under certain case, not sure it will be much more clean and 
won't need any control or stuff like that for signaling.


>    * I've also performed a quick review of the control implementation
>      and I've noticed some type problems:
>          o The counter variables aren't appropriate yet. LDB objects
>            are counted as "unsigned int". That means the following in
>            the downto manner: for (i >= ...num_values - 1; i !=
>            (unsigned) - 1; i--).
>          o On "for (i=0; i < rmd.ctr.ctr1.count; i++) {" "i" should be
>            "uint32_t" since we are working os DSR stuff.
>          o "functional_level" in "struct dirsync_control" should be
>            typed as "int".
>          o "addedAttributes" I would type as "unsigned int" (the same
>            as "num_elements" in LDB).
Mathias, thanks for the "review" that I didn't ask for but I didn't made 
the review for myself, I just pushed in my tree for 3 reasons:

* have a kind of remote backup
* be able to discuss with nadya on this control
* allow other to play with it

For the moment it's in the state where "it works" but I'm fully aware 
that the code is not 100% clean ...

Matthieu.
-- 

Matthieu Patou
Samba Team        http://samba.org
Private repo      http://git.samba.org/?p=mat/samba.git;a=summary