acess based share enum: handle permission set in configuration files

github at samba.org github at samba.org
Mon Feb 29 10:38:35 UTC 2016


New comment by bud4 on Samba Github repository

https://github.com/samba-team/samba/pull/54#issuecomment-190147183
Comment:
2016-02-28 13:28 GMT+01:00 Uri Simchoni <notifications at github.com>:

> 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.
>
>     sorry I did not see that the user_ok_token function returns true if in
the configuration file is not set any permission.


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

>
>    1.
>
>    We need a bug in bugzilla and a BUG: line pointing to it in the commit
>    message (see other commit messages in git log)
>
> ok if the bug is not present in bugzilla then I'll create it

>
>    1.
>
>    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:
>
> if (!user_ok_token(...)) {
>         return false;
> }
> return share_access_check(...);
>
> ok i will do it as soon as I can...
Bye
Alberto

Could you resubmit with those changes? I can later throw in some tests to
> verify correct behavior.
> Thanks,
> Uri
>
> —
> Reply to this email directly or view it on GitHub
> <https://github.com/samba-team/samba/pull/54#issuecomment-189857051>.
>



More information about the samba-technical mailing list