> -----Original Message-----
> From: justin.erenkrantz_at_gmail.com
>
> Sent: Mittwoch, 25. August 2010 08:14
> To: dev_at_httpd.apache.org
> Cc: Subversion Development
> Subject: 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.
It is needed for setting module specific log levels.
>
> 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
I admit the code is complex and it needs some time to understand it
especially without the parallel information that was part of the discussion
on the dev@ list. So improvement proposals to the documentation are welcome
to make things easier to understand.
Basicly all this stuff is done to use the advanced features of C99 compilers
that allow us to check with low effort whether a call to the error logging
function is needed at all given the currently configured loglevel.
This should improve performance, especially as the new logging API encourages
to log even more details as it is now possible to limit the level of log detail
to specific modules or even requests.
For the gory details the dev@ archives should be consulted. See various
threads in March, April and May that all have loglevel in the subject.
> 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.
The question is if we should consider APLOG_MARK as an isolated item
of the logging API or something that works only fine in conjunction with
the whole API.
When you get through the archives you will see that a lengthy discussion
with several patch iterations took place over several months in spring.
It was discussed specifically how to keep the pain level low for existing
modules that want
1. to use the new features of the API
2. that do not want to use the new features and just keep working as they are
by just recompiling them against trunk.
The outcome was the current solution that should work fine for 90% or more
of the existing modules.
Unfortunately mod_authz_svn falls into the 10% where it does not work painless.
We have not found a way to do this painlessly for modules that have something
like log_access_verdict. These modules need code changes, but as said these
were seen as the minor number of cases.
> 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.
Trunk has a major bump, a major bump was done for this specific change,
there is an entry in the changelog for 2.3.6. The only thing that is missing
is an entry in http://httpd.apache.org/docs/trunk/new_features_2_4.html
in Module Developer Changes section.
And yes it should be mentioned there with some hints.
But as a whole I think there are a lot of pointers to this API change.
>
> (I wish I could veto this whole commit just on over-complication alone
> - it's not an elegant solution at all.)
Please take yourself sometime to catch up with the discussion that took
place on the list and you might understand the reasoning and motivations better
(Which does not mean that you need to agree with tehm :-)).
Hopefully this does not come across as the RTFA advice as it is not.
It is just hard to present all facets of a monthlong discussion in this reply.
Regards
Rüdiger
Received on 2010-08-25 11:14:04 CEST