Here's my review notes:
* Use two spaces after periods in the log messages.
* I think we normally use "err", not "error", to name variables or
fields containing errors, but I might be making up the consistency of
that convention.
* You missed space-before-paren in a number of places.
* "Dependancy" isn't strictly 100% incorrect, but "dependency" is the
normal way it's written.
* You have "wether" in a number of places; it's spelled "whether".
(Including some places in existing code. Didn't we cover this last time
around?)
* Your design of authz file validation is a little dissatisfactory.
By performing the checking in authz_group_contains_user() and using that
function in authz_validate_group(), you've presented a structural
obstacle to taking advantage of the validation by not doing
error-checking during actual authz lookup.
* In fact, if the second patch were designed properly, the first patch
would be essentially unnecessary.
+ svn_config_get (b->config, &val, "groups",
+ &name[1], NULL);
Fits nicely on one line. Why split it?
+/** A data type specific to authz configurations. Consider as
+ opaque. */
+typedef svn_config_t svn_authz_t;
I'm not sure that this really allows us to change the representation of
svn_authz_t in the future. I'd really like to see:
typedef struct svn_authz_t svn_authz_t;
In authz.c, you can either define:
struct svn_authz_t {
svn_config_t *cfg;
};
Or, you can cast svn_authz_t * pointers to svn_config_t * pointers and
vice versa, and never actually make struct svn_authz_t a complete type.
ANSI C guarantees that pointer to struct foo can be casted to pointer to
struct bar without losing information, I believe, so this is correct if
perhaps a little weird. You'd only need to cast in two places, I think
(once when returning an svn_authz_t * in the read function, and once
when accepting it in the query function).
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Thu Jul 14 16:25:49 2005