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

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

From: Stefan Sperling <stsp_at_elego.de>
Date: Tue, 1 Mar 2011 14:00:07 +0100

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);
> }
Received on 2011-03-01 14:00:51 CET

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