acess based share enum: handle permission set in configuration files

github at samba.org github at samba.org
Sun Feb 28 12:30:07 UTC 2016


New comment by urisimchoni on Samba Github repository

https://github.com/samba-team/samba/pull/54#issuecomment-189857051
Comment:
Hi Alberto,
Thanks for submitting this fix.
Some comments:

1. The checks for lp_valid_users and lp_invalid_users seem redundant - they are being done inside user_ok_token() without extra cost, it also seems that the proposed patch misses lp_only_user() this way.

2. The "username" parameter to user_ok_token() needs to be session_info->unix_info->unix_name (that's what you exchange for %U)

3. We need a bug in bugzilla and a BUG: line pointing to it in the commit message (see other commit messages in git log)

4. Please consult README.Coding - surround if blocks with curly braces. The preferred coding style also prefers multiple simple if statements over a complex condition, if possible. Something like:
```c
if (!user_ok_token(...)) {
        return false;
}

return share_access_check(...);
```

Could you resubmit with those changes? I can later throw in some tests to verify correct behavior.
Thanks,
Uri


More information about the samba-technical mailing list