kfogel@collab.net wrote:
>>--- subversion/libsvn_repos/repos.h (/online) (revision 443)
>>+++ subversion/libsvn_repos/repos.h (/local/authz-in-svnserve) (revision 443)
>>@@ -70,6 +70,7 @@
>> /* In the repository conf directory, look for these files. */
>> #define SVN_REPOS__CONF_SVNSERVE_CONF "svnserve.conf"
>> #define SVN_REPOS__CONF_PASSWD "passwd"
>>+#define SVN_REPOS__CONF_AUTHZ "authz"
>>
>>
>This isn't really due to your change, but I find both the comment and
>the names of these constants a bit misleading. These files are just
>what is looked for *by default*, right? Depending on how the admin
>configures things for run-time, totally different files might be
>looked for. I wish the comment and the names expressed this better.
>
>
IIUC, CONF_PASSWD and CONF_AUTHZ are default values for svn_config_read,
whilst CONF_SVNSERVE_CONF is the name of the config file. So you can't
really use the same comment for all three, and the first two should have
a DEFAULT in the name somewhere.
(We can change the names, these being private symbols.)
>>+/* Ensure that the client has the REQUIRED_ACCESS by checking the
>>+ * blanket access directives and possibly authz directives if PATH is
>>+ * not NULL.
>>+ */
>>+static svn_boolean_t lookup_access(apr_pool_t *pool,
>>+ server_baton_t *b,
>>+ svn_repos_authz_access_t required,
>>+ const char *path,
>>+ svn_boolean_t needs_username)
>>
>>
>
>The parameter's name is REQUIRED, not REQUIRED_ACCESS. How is the
>NEEDS_USERNAME parameter used? The doc string doesn't mention it, nor
>does it mention B, though B's function can be deduced more easily
>(it's the source of the "authz directives" referred to earlier,
>presumably).
>
>
And woldn't it be nice if "b" were called "baton" instead.
Single-character variables make my froodleprog itch.
-- Brane
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri Aug 26 00:30:19 2005