On 7/19/05, Brian Holmes <svndev@gmail.com> wrote:
> 
> 
> On 7/15/05, Greg Hudson <ghudson@mit.edu> wrote:
> > 
> > On Sat, 2005-07-16 at 01:34 +0200, Branko Èibej wrote:
> > > #define SVN_REPOS_WRITE_ERRORLOG(repo, level, process, message, pool) 
> > \
> > > while { \
> > > svn_repos_loglevel_t loglevel; \
> > > SVN_ERR (svn_repos_get_loglevel (&loglevel, (repo), (pool))); \ 
> > > if ((level) <= loglevel) \
> > > SVN_ERR (svn_repos_write_errorlog ((repo), (level), (process), \
> > > (message), (pool))); \
> > > } while (0)
> > 
> > Sorry if I missed this before.
> > 
> > Well, we're back to my distaste for non-trivial macros.
> 
> 
> Perhaps more convincingly, if the write_errorlog() function takes a
> > variable argument list as I suggested, we can't wrap that in a macro
> > without C99 tricks or an extra set of parentheses (and even that trick
> > won't work right, since we need to get at one of the arguments).
> 
> 
> 
> I'm in agreement with Brane on the macros. Personally I have no qualms 
> with macros if they are used carefully. I think in this case, what macros 
> buy us far outweighs their evils.
> 
> First of all, lets clean up this macro with the most recent changes:
> 
> #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.
> * 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.
> 
> Logging is one place where I believe macros perform beautifully. 
> 
> 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?
> 
> - Brian Holmes
> 
> 
> ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> > For additional commands, e-mail: dev-help@subversion.tigris.org
> > 
> > 
> 
> 
Oops, that macro needs protection:
#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);
Received on Wed Jul 20 23:06:41 2005