Re: [PATCH] Compiling subversion trunk with httpd trunk code fails

Herewith attached the updated patch and log message.

Thanks & Regards,
Vijayaguru

On Tuesday 01 March 2011 06:30 PM, Stefan Sperling wrote:
> On Tue, Mar 01, 2011 at 11:59:28AM +0530, vijay wrote:
>> [[[
>> Update log_access_verdict to make it work with httpd trunk as well as older
>> versions. The function is being called with APLOG_MACRO in few places.
>> The macro APLOG_MARK expands to 2 arguments till httpd-2.2.x but 3
>> arguments in httpd-2.3-dev, which causes failure while compiling with
>> httpd-2.3-dev. So we need to handle both the cases.
>>
>> * subversion/mod_authz_svn/mod_authz_svn.c
>> (log_access_verdict): Modify the signature.
> Your log message should probably include this link:
> http://httpd.apache.org/docs/trunk/developer/new_api_2_4.html#upgrading_logging
>
>> Patch by: Vijayaguru G<vijay{_AT_}collab.net>
>> Suggested by: Kameshj
>> ]]]
>>
>>
>> Index: subversion/mod_authz_svn/mod_authz_svn.c
>> ===================================================================
>> --- subversion/mod_authz_svn/mod_authz_svn.c (revision 1075316)
>> +++ subversion/mod_authz_svn/mod_authz_svn.c (working copy)
>> @@ -32,6 +32,7 @@
>> #include<http_log.h>
>> #include<ap_config.h>
>> #include<ap_provider.h>
>> +#include<ap_release.h>
>> #include<apr_uri.h>
>> #include<apr_lib.h>
>> #include<mod_dav.h>
>> @@ -519,12 +520,20 @@
>> return OK;
>> }
>>
>> +#if AP_SERVER_MAJORVERSION_NUMBER == 2&& AP_SERVER_MINORVERSION_NUMBER == 3
>> +#define LOG_ARGS_SIGNATURE const char *file, int line, int module_index
>> +#define LOG_ARGS_CASCADE file, line, module_index
>> +#else
>> +#define LOG_ARGS_SIGNATURE const char *file, int line
>> +#define LOG_ARGS_CASCADE file, line
>> +#endif
> I don't really like a macro that expands to function parameters.
> Can we do something like this instead?
> This will duplicate some code, but the function is small and this way
> it's a bit easier to understand what is happening at a glance.
>
> #if AP_MODULE_MAGIC_AT_LEAST(2,3)
> static void
> log_access_verdict_httpd_v23(const char *file, int line,
> const request_rec *r, int allowed,
> const char *repos_path,
> const char *dest_repos_path)
> {
> /* code for v23 */
> }
> #else
> static void
> log_access_verdict2_httpd_v22(const char *file, int line, int module_index,
> const request_rec *r, int allowed,
> const char *repos_path, const char *dest_repos_path)
> {
> /* code for v22 */
> }
> #endif
>
> static void
> log_access_verdict(const char *file, int line, int module_index,
> const request_rec *r, int allowed,
> const char *repos_path, const char *dest_repos_path)
> {
> #if AP_MODULE_MAGIC_AT_LEAST(2,3)
> /* call log_access_verdict_httpd_v23 */
> #endif
> /* call log_access_verdict_httpd_v22 */
> #endif
> }
>
>> /* Log a message indicating the access control decision made about a
>> - * request. FILE and LINE should be supplied via the APLOG_MARK macro.
>> - * ALLOWED is boolean. REPOS_PATH and DEST_REPOS_PATH are information
>> + * request. LOG_ARGS_SIGNATURE should be supplied via the APLOG_MARK macro
>> + * with respect to Apache version. ALLOWED is boolean.
>> + * REPOS_PATH and DEST_REPOS_PATH are information
>> * about the request. DEST_REPOS_PATH may be NULL. */
>> static void
>> -log_access_verdict(const char *file, int line,
>> +log_access_verdict(LOG_ARGS_SIGNATURE,
>> const request_rec *r, int allowed,
>> const char *repos_path, const char *dest_repos_path)
>> {
>> @@ -534,22 +543,22 @@
>> if (r->user)
>> {
>> if (dest_repos_path)
>> - ap_log_rerror(file, line, level, 0, r,
>> + ap_log_rerror(LOG_ARGS_CASCADE, level, 0, r,
>> "Access %s: '%s' %s %s %s", verdict, r->user,
>> r->method, repos_path, dest_repos_path);
>> else
>> - ap_log_rerror(file, line, level, 0, r,
>> + ap_log_rerror(LOG_ARGS_CASCADE, level, 0, r,
>> "Access %s: '%s' %s %s", verdict, r->user,
>> r->method, repos_path);
>> }
>> else
>> {
>> if (dest_repos_path)
>> - ap_log_rerror(file, line, level, 0, r,
>> + ap_log_rerror(LOG_ARGS_CASCADE, level, 0, r,
>> "Access %s: - %s %s %s", verdict,
>> r->method, repos_path, dest_repos_path);
>> else
>> - ap_log_rerror(file, line, level, 0, r,
>> + ap_log_rerror(LOG_ARGS_CASCADE, level, 0, r,
>> "Access %s: - %s %s", verdict,
>> r->method, repos_path);
>> }