httpd trunk broken with Subversion: ap_log_rerror busted

In r951893, httpd modified a #define for APLOG_MARK to add in a new
parameter called APLOG_MODULE_INDEX (in addition to file and line
info).

This busts Subversion - specifically, mod_authz_svn which has a function called:

static void
log_access_verdict(const char *file, int line,
                   const request_rec *r, int allowed,
                   const char *repos_path, const char *dest_repos_path)

it is called with:

  log_access_verdict(APLOG_MARK, r, 1, repos_path, dest_repos_path);

Reading the very obtuse (unhelpful) commit log for r951893 as well as
any comments in that general area of http_log.h (which are lacking &
unhelpful), I have no idea what this APLOG_MODULE_INDEX is or why it's
even there.

Furthermore, this error case is very very hard to track down because
we're relying upon multiple levels of #define's and indirections
(hidden static variables??, weird STDC wrappers, redefinitions of
function names, etc, etc.). So, it's not going to be obvious to
downstream developers what is going on and why it is broken. In
general, I'm not quite sure it's a good idea to bust a #define that
has been the same since the 1.3 days (if not earlier actually). If we
want modules to use a new optimized log API, then we should introduce
a new optimized log API and not bust the old one in a way that the
failure case isn't obvious to track down.

Yes, we could fix this by making mod_authz_svn conditional on the new
MMN, but - again, it's about even figuring out that the API is changed
and what to do about it. The root cause is just way too obscured,
IMO.

(I wish I could veto this whole commit just on over-complication alone
- it's not an elegant solution at all.)

My $.02. -- justin
Received on 2010-08-25 08:15:11 CEST