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

Re: libsvn_repos logging -- design problems

From: Greg Hudson <ghudson_at_mit.edu>
Date: 2005-07-20 06:16:12 CEST

On Tue, 2005-07-19 at 20:08 -0700, Brian Holmes wrote:
> #define svn_repos_write_errorlog(repos, log_level, message) \
> do { \
> if ((repos)->error_log->level <= log_level) \
> svn_log_write((repos)->error_log, log_level, message) \
> } while(0);
>
> The value of the macro here is huge. The macro allows us to:
> * enforce a constant method of logging that checks the log level
> before calling the function
> * prevents us from processing the 'message' parameter when
> unnecessary.
> * makes our code much more readable.
> * makes logging simple from a programmer perspective while keeping
> things efficient.

You've managed to say two things in four ways, and one of them isn't
true. Because the macro sacrifices varargs, our code looks less
readable:

  SVN_REPOS_LOG(repos, log_level, apr_psprintf(pool, fmt, args), pool)

is less readable than

  svn_repos_log(repos, log_level, pool, fmt, args)

Your macro also requires us to expose the internals of svn_repos_t in
the ABI; svn_repos_t is currently an opaque type.

> * we can easily make changes to logic by changing the macro (like
> checking if repos or repos->error_log is null before calling the
> function) or we could completely (and possibly selectively) remove the
> logging code altogether.

We can easily make changes to the logic of the function too. Macros are
not superior to functions as a means of abstraction.

> I'm not sure what to say about the var args. It is true that using a
> macro will not allow us to use var args cleanly. However, do we
> really need to use varargs and does it outweight my purported
> benefits?

For the kind of logging we will actually be adding (and so far, no one
has had a *single* counterexample to this), the purported benefits allow
us to save a few cycles before executing an operation such as "checkout"
which will take millions upon millions of cycles to execute.

Compared to that, the readability of a varargs function is orders of
magnitude more important.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Jul 20 23:06:33 2005

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