[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-02 22:55:55 CEST

Here's what I found:

  1. You're using tabs. We don't use tabs in the Subversion code base
(at least not in the C code).

  2. The access types enum should be in svn_repos.h, not svn_types.h.
There's no rule that says all types are defined in svn_types.h, despite
the name.

  3. I'm dubious about AUTHZ_ACCESS_GRANTED and AUTHZ_ACCESS_DETERMINED
being defined as macros. You can't debug into a macro, so defining big
macros tends to reduce our code's maintainability. I would define these
as functions unless you can measure a performance difference from
defining them as macros.

  4. Although the decision to use space-before paren is supposedly
per-file, and this is a new file, I think introducing a
no-space-before-paren file into libsvn_repos would generate a lot of
confusion.

  5. I can't tell what changes you made between the old mod_authz_svn
code and the new libsvn_repos code. Clearly there are some substantial
changes, since the macros I mentioned in (3) above don't exist in the
old code. Your log message should provide some guidance.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sat Jul 2 22:58:10 2005

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