On Thu, 2005-07-21 at 15:06 +0200, David Anderson wrote:
+/** Read authz configuration data from @a file (a file or registry
+ * path) into @a *cfgp, allocated in @a pool.
*cfgp should be *authzp.
- * Check wether @a user can access @a path in the repository @a
+ * Check whether @a user can access @a path in the repository @a
* repos_name with the @a required_access. @a cfg lists the ACLs to
cfg should have been changed to authz.
+ /* The authz rules for the phase 2 tests, second case (missing group
+ definition). */
As far as I can tell, you still have the cyclic dependency from the
first case in these rules, making this a fragile test.
+static svn_error_t *
+authz_group_contains_user (svn_config_t *cfg,
Why didn't you leave this returning a boolean? You seem to have left in
some of the internal structure for returning errors from
svn_repos_authz_check_access(), which is unnecessary gunk in the current
implementation since it never actually returns an error. Perhaps you
reasoned that the internals of authz.c should leave the door open to
returning errors in the future, in which case I do not agree. While I
think the API should leave the door open to that possibility, any good
implementation will not return errors; thus, the internal logic does not
need to contain gunk to that end.
I advise reverting the function signature here, reverting the addition
of the err field to the baton, reverting the call site in
authz_parse_line(), reverting the signature of authz_get_tree_access and
the changes to that function, reverting the signature of
authz_get_path_access and the changes to that function, and reverting
the call sites to the two get_*_access functions in
svn_repos_authz_check_access().
(I do like the simplification where checked_groups is no longer
necessary. Less memory allocation in svn_repos_authz_check_access() is
pleasing, although of course we still have some left in
authz_group_contains_user.)
The rest of the patch (the validation checking) looks great. One more
round and I think we should be good.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sat Jul 23 15:12:18 2005