Re: [PATCH] libsvn_repos logging API -- first draft

The only thing I noticed was some apr_file_t * parameters. We always
wind up wishing we used svn_stream_t *s when we do that.

I'll also reiterate the point that repository-level logging can't
capture events which don't involve a repository, such as a client trying
to connect to a URL which doesn't successfully map to a repository.

On Fri, 2005-07-15 at 06:42 +0200, Branko Čibej wrote:
> Hm. I wonder, wouldn't it be easier if you had an
> svn_repos_set_process_name, and ommitted the process_name parameter
> everywhere?

Agreed.

> One thing about this API has me worried, and that's the assumption that
> you have to make a function call in order to check the log level

Presumably you can cache it in the caller, who can keep track of when it
resets the log level. (I'm not even sure if we really need a getter
function, really, but it's low-overhead.)

> Really, what are you going to do if your get_loglevel or write_errorlog
> error out? Crash the server? I don't think so. By all means error out if
> you can't open the log file, that usually happens at process startup.

Agreed. It's bad if you run out of disk space writing log entries, but
what are you going to do?

> This brings us to the accessor functions. As I understand it,
> svn_repos_(get|set)_loglevel and svn_repos_(get|set)_(access|error)log
> somply manipulate fields within the svn_repos_t. I fail to see how these
> functions can fail (unless the svn_repos_t is NULL), or when they'd ever
> need a pool -- any allocations they _do_ need can come from the
> svn_repos_t's pool.

Agreed.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri Jul 15 07:01:21 2005