[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH] Move authz access control to libsvn_repos

From: Greg Hudson <ghudson_at_MIT.EDU>
Date: 2005-07-03 16:40:16 CEST

Okay, it's looking pretty good now. A few new issues I've found:

  * build.conf currently only makes mod_authz_svn depend on libsvn_subr.
You'll need to change that to make it depend on libsvn_repos, I believe.

  * Your addition to repos-test.c opens a file /tmp/authz.tmp, which is
not portable to Windows and is probably a security issue on Unix. You
should be using apr_file_mktemp(); see tests/libsvn_delta/random-test.c
for an example.

  * I think we generally put two spaces after sentence-ending periods in
log messages and doc strings. (Except when it's the end of a comment,
in which case one space before the "*/" is fine.) You're only putting
one space after them.

  * What kind of testing have you done with mod_authz_svn in particular,
to make sure that I won't break it when I commit this?

+ /** no access */
+ svn_authz_none = 0,

"No access"

+ * Check wether @a user can access @a path in the repository @a

"Check whether". You mispell "whether" in five other places in the
patch as well.

+/* Test that authz is giving out the right authorizations */
+ /* Definition of the paths to test and expected replies for each */
+ /* Output the authz file to disk */
+ /* Read the authz configuration back and start testing */

Comments which are sentences should end with a period. (Not sure if
Subversion is consistent about that; it's a personal assertion.) There
are some more examples inside authz.c.

+/* Determine wether the required access is granted given what authz *

I think the final '*' is spurious there.

--- subversion/include/svn_repos.h (révision 15231)
+++ subversion/include/svn_repos.h (copie de travail)
@@ -30,8 +30,8 @@
 #include "svn_types.h"
 #include "svn_error.h"
 #include "svn_version.h"
+#include "svn_config.h"
 
-

Spurious deletion of a blank line there.

+ /* non-anon rule, anon user. Stop. */
+ /* group rule and user not in group. Stop. */
+ /* user rule for wrong user. Stop. */

Capitalize.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sun Jul 3 16:42:14 2005

This is an archived mail posted to the Subversion Dev mailing list.