kfogel@collab.net wrote:
> ghudson@tigris.org writes:
>
>>+/* Examine the entire definition of a group, checking for errors. The
>>+ * errors detected are references to non-existent groups and circular
>>+ * dependencies between groups. */
>>+static svn_error_t *
>>+authz_group_walk (svn_config_t *cfg,
>>+ const char *group,
>>+ apr_hash_t *checked_groups,
>>+ apr_pool_t *pool)
[...]
>>+{
>>+ const char *value;
>>+ apr_array_header_t *list;
>>+ int i;
>>+
>>+ svn_config_get (cfg, &value, "groups", group, NULL);
[...]
>>+ /* Having a non-existent group in the ACL configuration might be the
>>+ sign of a typo. Refuse to perform authz on uncertain rules. */
>>+ if (!value)
>>+ return svn_error_createf (SVN_ERR_AUTHZ_INVALID_CONFIG, NULL,
>>+ "An authz rule refers to group '%s', "
>>+ "which is undefined",
>>+ group);
>>+
>>+ list = svn_cstring_split (value, ",", TRUE, pool);
>>+
>>+ for (i = 0; i < list->nelts; i++)
>>+ {
>>+ const char *group_user = APR_ARRAY_IDX(list, i, char *);
>>+
>>+ /* If the 'user' is a subgroup, recurse into it. */
>>+ if (*group_user == '@')
>>+ {
>>+ /* A circular dependency between groups is a Bad Thing. We
>>+ don't do authz with invalid ACL files. */
>>+ if (apr_hash_get (checked_groups, &group_user[1],
>>+ APR_HASH_KEY_STRING))
>>+ return svn_error_createf (SVN_ERR_AUTHZ_INVALID_CONFIG,
>>+ NULL,
>>+ "Circular dependency between "
>>+ "groups '%s' and '%s'",
>>+ &group_user[1], group);
>>+
>>+ /* Add group to hash of checked groups. */
>>+ apr_hash_set (checked_groups, &group_user[1],
>>+ APR_HASH_KEY_STRING, "");
>>+
>>+ /* Recurse on that group. */
>>+ SVN_ERR (authz_group_walk (cfg, &group_user[1],
>>+ checked_groups, pool));
>>+ }
>>+ }
>>+
>>+ return SVN_NO_ERROR;
>>+}
>
> We have a general problem (well, problemlet) in Subversion that when
> iteration is done via recursion, our usual loop subpool idiom doesn't
> work.
Is this "iteration done via recursion"? It looks to me like iteration done by
a loop, plus recursion done by recursion, but then I'm not familiar with the
config file format being parsed so I may have misunderstood it.
Does your comment that our usual loop subpool idiom wouldn't work apply in this
case? The pool memory used by the checked_groups hash would be unaffected (as
all additions go into its own pool), but the pool memory used by
authz_group_walk() (which appears to be only that used by "list =
svn_cstring_split()") would be bounded by the depth of recursion.
In practice there's probably no point given the size of most config files, but
what am I missing in theory?
> I don't have anything constructive to suggest, though:
> recursion is obviously the right strategy here, and the acrobatics
> necessary to get bounded subpool usage wouldn't be worth it, given the
> size of most config files.
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Jul 26 21:51:12 2005