Comments on http://repo.or.cz/w/Samba/mdw.git?a=shortlog; h=refs/heads/transGroups

[Stefan (metze) Metzmacher]

Hi Matthias,

I just mean that

 if (sid == NULL) {
      return NT_STATUS_OK;
 }

 already_there = sids_contains_sid((const struct dom_sid**) *res_sids,
                                   *num_res_sids, sid);
 if (already_there) {
      return NT_STATUS_OK;
 }

would do the same, but is more readable.

I mostly follow the rule to do only one statement in a line.

1.) assing the return values of a function to variables

or

2.) check some logic.

I think that makes it easier to follow the logic.

I also always review my own patches, like I would review patches from
others, typically a day or some hours later.
(git rebase -i is my best friend :-)

I also wonder why you add the new code to sidmap.c.
I'd prefer if use just make the new function static in
source4/auth/sam.c. And I would rename calc_trans_clos_sid()
to something like authsam_expand_nested_groups() or
authsam_expand_memberships()

the code is sidmap.c is not used anymore, and I think I'll remove it
(at from the build) soon.

metze

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 252 bytes
Desc: OpenPGP digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20090730/edac5066/attachment.pgp>