httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jan Kalu┼ża <jkal...@redhat.com>
Subject Re: svn commit: r1525597 - in /httpd/httpd/trunk: CHANGES docs/manual/mod/core.xml include/http_core.h include/httpd.h server/core.c server/log.c
Date Tue, 24 Sep 2013 11:32:53 GMT
On 09/23/2013 08:39 PM, Jeff Trawick wrote:
> On Mon, Sep 23, 2013 at 10:02 AM, <jkaluza@apache.org
> <mailto:jkaluza@apache.org>> wrote:
>
>     Author: jkaluza
>     Date: Mon Sep 23 14:02:27 2013
>     New Revision: 1525597
>
>     URL: http://svn.apache.org/r1525597
>     Log:
>     Add ap_errorlog_provider to make ErrorLog logging modular. Move
>     syslog support from core to new mod_syslog.
>
>
> Since new fields were added to the middle of existing structures, you
> need a major bump to MMN so that existing modules won't load without
> recompile.

Thanks for reviewing my patches. I've fixed the problems you have found 
in r1525845.

Regards,
Jan Kaluza

> Here's an example of a major bump:
>
> http://svn.apache.org/viewvc/httpd/httpd/trunk/include/ap_mmn.h?r1=1496709&r2=1498880&diff_format=h
>
>
>     Modified:
>          httpd/httpd/trunk/CHANGES
>          httpd/httpd/trunk/docs/manual/mod/core.xml
>          httpd/httpd/trunk/include/http_core.h
>          httpd/httpd/trunk/include/httpd.h
>          httpd/httpd/trunk/server/core.c
>          httpd/httpd/trunk/server/log.c
>
>     Modified: httpd/httpd/trunk/CHANGES
>     URL:
>     http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=1525597&r1=1525596&r2=1525597&view=diff
>     ==============================================================================
>     --- httpd/httpd/trunk/CHANGES [utf-8] (original)
>     +++ httpd/httpd/trunk/CHANGES [utf-8] Mon Sep 23 14:02:27 2013
>     @@ -1,6 +1,9 @@
>                                                                -*-
>     coding: utf-8 -*-
>       Changes with Apache 2.5.0
>
>     +  *) core: Add ap_errorlog_provider to make ErrorLog logging
>     modular. Move
>     +     syslog support from core to new mod_syslog. [Jan Kaluza]
>     +
>         *) mod_proxy_fcgi: Handle reading protocol data that is split
>     between
>            packets.  [Jeff Trawick]
>
>
>     Modified: httpd/httpd/trunk/docs/manual/mod/core.xml
>     URL:
>     http://svn.apache.org/viewvc/httpd/httpd/trunk/docs/manual/mod/core.xml?rev=1525597&r1=1525596&r2=1525597&view=diff
>     ==============================================================================
>     --- httpd/httpd/trunk/docs/manual/mod/core.xml (original)
>     +++ httpd/httpd/trunk/docs/manual/mod/core.xml Mon Sep 23 14:02:27 2013
>     @@ -1298,16 +1298,19 @@ ErrorDocument 404 /cgi-bin/bad_urls.pl
>     <http://bad_urls.pl>
>           more information.</p>
>
>           <p>Using <code>syslog</code> instead of a filename enables
logging
>     -    via syslogd(8) if the system supports it. The default is to use
>     -    syslog facility <code>local7</code>, but you can override this by
>     -    using the <code>syslog:<var>facility</var></code> syntax
where
>     -    <var>facility</var> can be one of the names usually documented in
>     +    via syslogd(8) if the system supports it and if
>     <module>mod_syslog</module>
>     +    is loaded. The default is to use syslog facility
>     <code>local7</code>,
>     +    but you can override this by using the
>     <code>syslog:<var>facility</var></code>
>     +    syntax where <var>facility</var> can be one of the names
>     usually documented in
>           syslog(1).  The facility is effectively global, and if it is
>     changed
>           in individual virtual hosts, the final facility specified
>     affects the
>           entire server.</p>
>
>           <highlight language="config">ErrorLog syslog:user</highlight>
>
>     +    <p>Additional modules can provide their own ErrorLog providers.
>     The syntax
>     +    is similar to <code>syslog</code> example above.</p>
>     +
>           <p>SECURITY: See the <a
>           href="../misc/security_tips.html#serverroot">security tips</a>
>           document for details on why your security could be compromised
>
>     Modified: httpd/httpd/trunk/include/http_core.h
>     URL:
>     http://svn.apache.org/viewvc/httpd/httpd/trunk/include/http_core.h?rev=1525597&r1=1525596&r2=1525597&view=diff
>     ==============================================================================
>     --- httpd/httpd/trunk/include/http_core.h (original)
>     +++ httpd/httpd/trunk/include/http_core.h Mon Sep 23 14:02:27 2013
>     @@ -833,8 +833,8 @@ typedef struct ap_errorlog_info {
>           /** apr error status related to the log message, 0 if no error */
>           apr_status_t status;
>
>     -    /** 1 if logging to syslog, 0 otherwise */
>     -    int using_syslog;
>     +    /** 1 if logging using provider, 0 otherwise */
>     +    int using_provider;
>           /** 1 if APLOG_STARTUP was set for the log message, 0 otherwise */
>           int startup;
>
>     @@ -842,6 +842,30 @@ typedef struct ap_errorlog_info {
>           const char *format;
>       } ap_errorlog_info;
>
>     +#define AP_ERRORLOG_PROVIDER_GROUP "error_log_writer"
>     +#define AP_ERRORLOG_PROVIDER_VERSION "0"
>     +#define AP_ERRORLOG_DEFAULT_PROVIDER "file"
>     +
>     +typedef struct ap_errorlog_provider ap_errorlog_provider;
>     +
>     +struct ap_errorlog_provider {
>     +    /** Initializes the error log writer.
>     +     * @param p The pool to create any storage from
>     +     * @param s Server for which the logger is initialized
>     +     * @return Pointer to handle passed later to writer() function
>     +     */
>     +    void * (*init)(apr_pool_t *p, server_rec *s);
>     +
>     +    /** Logs the error message to external error log.
>     +     * @param info Context of the error message
>     +     * @param handle Handle created by init() function
>     +     * @param errstr Error message
>     +     * @param len Length of the error message
>     +     */
>     +    apr_status_t (*writer)(const ap_errorlog_info *info, void *handle,
>     +                           const char *errstr, int len);
>     +};
>     +
>       /**
>        * callback function prototype for a external errorlog handler
>        * @note To avoid unbounded memory usage, these functions must not
>     allocate
>
>     Modified: httpd/httpd/trunk/include/httpd.h
>     URL:
>     http://svn.apache.org/viewvc/httpd/httpd/trunk/include/httpd.h?rev=1525597&r1=1525596&r2=1525597&view=diff
>     ==============================================================================
>     --- httpd/httpd/trunk/include/httpd.h (original)
>     +++ httpd/httpd/trunk/include/httpd.h Mon Sep 23 14:02:27 2013
>     @@ -1245,6 +1245,10 @@ struct server_rec {
>           apr_file_t *error_log;
>           /** The log level configuration */
>           struct ap_logconf log;
>     +    /** External error log writer provider */
>     +    struct ap_errorlog_provider *errorlog_provider;
>     +    /** Handle to be passed to external log provider's logging
>     method */
>     +    void *errorlog_provider_handle;
>
>           /* Module-specific configuration for server, and defaults... */
>
>
>     Modified: httpd/httpd/trunk/server/core.c
>     URL:
>     http://svn.apache.org/viewvc/httpd/httpd/trunk/server/core.c?rev=1525597&r1=1525596&r2=1525597&view=diff
>     ==============================================================================
>     --- httpd/httpd/trunk/server/core.c (original)
>     +++ httpd/httpd/trunk/server/core.c Mon Sep 23 14:02:27 2013
>     @@ -48,6 +48,7 @@
>       #include "mod_core.h"
>       #include "mod_proxy.h"
>       #include "ap_listen.h"
>     +#include "ap_provider.h"
>
>       #include "mod_so.h" /* for ap_find_loaded_module_symbol */
>
>     @@ -3955,6 +3956,49 @@ static apr_array_header_t *parse_errorlo
>           return a;
>       }
>
>     +static const char *set_errorlog(cmd_parms *cmd, void *dummy, const
>     char *arg1,
>     +                                const char *arg2)
>     +{
>     +    ap_errorlog_provider *provider;
>     +    cmd->server->errorlog_provider = NULL;
>     +
>     +    if (!arg2) {
>     +        /* Stay backward compatible and check for "syslog" */
>     +        if (strncmp("syslog", arg1, 6) == 0) {
>     +            arg2 = arg1 + 7; /* skip the ':' if any */
>     +            arg1 = "syslog";
>     +        }
>     +        else {
>     +            /* Admin can define only "ErrorLog provider" and we should
>     +             * still handle that using the defined provider, but
>     with empty
>     +             * error_fname. */
>     +            provider =
>     ap_lookup_provider(AP_ERRORLOG_PROVIDER_GROUP, arg1,
>     +
>       AP_ERRORLOG_PROVIDER_VERSION);
>     +            if (provider) {
>     +                arg2 = "";
>     +            }
>     +            else {
>     +                return set_server_string_slot(cmd, dummy, arg1);
>     +            }
>     +        }
>     +    }
>     +
>     +    if (strcmp("file", arg1) == 0) {
>     +        return set_server_string_slot(cmd, dummy, arg2);
>     +    }
>     +
>     +    provider = ap_lookup_provider(AP_ERRORLOG_PROVIDER_GROUP, arg1,
>     +                                    AP_ERRORLOG_PROVIDER_VERSION);
>     +    if (!provider) {
>     +        return apr_psprintf(cmd->pool,
>     +                            "Unknown ErrorLog provider: %s",
>     +                            arg1);
>     +    }
>     +
>     +    cmd->server->errorlog_provider = provider;
>     +    return set_server_string_slot(cmd, dummy, arg2);
>     +}
>     +
>       static const char *set_errorlog_format(cmd_parms *cmd, void *dummy,
>                                              const char *arg1, const
>     char *arg2)
>       {
>     @@ -4118,7 +4162,7 @@ AP_INIT_TAKE1("ServerRoot", set_server_r
>         "Common directory of server-related files (logs, confs, etc.)"),
>       AP_INIT_TAKE1("DefaultRuntimeDir", set_runtime_dir, NULL,
>     RSRC_CONF | EXEC_ON_READ,
>         "Common directory for run-time files (shared memory, locks, etc.)"),
>     -AP_INIT_TAKE1("ErrorLog", set_server_string_slot,
>     +AP_INIT_TAKE12("ErrorLog", set_errorlog,
>         (void *)APR_OFFSETOF(server_rec, error_fname), RSRC_CONF,
>         "The filename of the error log"),
>       AP_INIT_TAKE12("ErrorLogFormat", set_errorlog_format, NULL, RSRC_CONF,
>     @@ -4560,7 +4604,7 @@ AP_DECLARE(int) ap_sys_privileges_handle
>       static int check_errorlog_dir(apr_pool_t *p, server_rec *s)
>       {
>           if (!s->error_fname || s->error_fname[0] == '|'
>     -        || strcmp(s->error_fname, "syslog") == 0) {
>     +        || s->errorlog_provider != NULL) {
>               return APR_SUCCESS;
>           }
>           else {
>     @@ -4986,7 +5030,7 @@ static void core_dump_config(apr_pool_t
>           apr_file_printf(out, "ServerRoot: \"%s\"\n", ap_server_root);
>           tmp = ap_server_root_relative(p, sconf->ap_document_root);
>           apr_file_printf(out, "Main DocumentRoot: \"%s\"\n", tmp);
>     -    if (s->error_fname[0] != '|' && strcmp(s->error_fname,
>     "syslog") != 0)
>     +    if (s->error_fname[0] != '|' && s->errorlog_provider == NULL)
>
>
> Check first for s->error_fname != NULL?
>
>               tmp = ap_server_root_relative(p, s->error_fname);
>           else
>               tmp = s->error_fname;
>
>     Modified: httpd/httpd/trunk/server/log.c
>     URL:
>     http://svn.apache.org/viewvc/httpd/httpd/trunk/server/log.c?rev=1525597&r1=1525596&r2=1525597&view=diff
>     ==============================================================================
>     --- httpd/httpd/trunk/server/log.c (original)
>     +++ httpd/httpd/trunk/server/log.c Mon Sep 23 14:02:27 2013
>     @@ -53,6 +53,7 @@
>       #include "http_main.h"
>       #include "util_time.h"
>       #include "ap_mpm.h"
>     +#include "ap_provider.h"
>
>       #if HAVE_GETTID
>       #include <sys/syscall.h>
>     @@ -75,71 +76,6 @@ APR_HOOK_STRUCT(
>
>       int AP_DECLARE_DATA ap_default_loglevel = DEFAULT_LOGLEVEL;
>
>     -#ifdef HAVE_SYSLOG
>     -
>     -static const TRANS facilities[] = {
>     -    {"auth",    LOG_AUTH},
>     -#ifdef LOG_AUTHPRIV
>     -    {"authpriv",LOG_AUTHPRIV},
>     -#endif
>     -#ifdef LOG_CRON
>     -    {"cron",    LOG_CRON},
>     -#endif
>     -#ifdef LOG_DAEMON
>     -    {"daemon",  LOG_DAEMON},
>     -#endif
>     -#ifdef LOG_FTP
>     -    {"ftp", LOG_FTP},
>     -#endif
>     -#ifdef LOG_KERN
>     -    {"kern",    LOG_KERN},
>     -#endif
>     -#ifdef LOG_LPR
>     -    {"lpr", LOG_LPR},
>     -#endif
>     -#ifdef LOG_MAIL
>     -    {"mail",    LOG_MAIL},
>     -#endif
>     -#ifdef LOG_NEWS
>     -    {"news",    LOG_NEWS},
>     -#endif
>     -#ifdef LOG_SYSLOG
>     -    {"syslog",  LOG_SYSLOG},
>     -#endif
>     -#ifdef LOG_USER
>     -    {"user",    LOG_USER},
>     -#endif
>     -#ifdef LOG_UUCP
>     -    {"uucp",    LOG_UUCP},
>     -#endif
>     -#ifdef LOG_LOCAL0
>     -    {"local0",  LOG_LOCAL0},
>     -#endif
>     -#ifdef LOG_LOCAL1
>     -    {"local1",  LOG_LOCAL1},
>     -#endif
>     -#ifdef LOG_LOCAL2
>     -    {"local2",  LOG_LOCAL2},
>     -#endif
>     -#ifdef LOG_LOCAL3
>     -    {"local3",  LOG_LOCAL3},
>     -#endif
>     -#ifdef LOG_LOCAL4
>     -    {"local4",  LOG_LOCAL4},
>     -#endif
>     -#ifdef LOG_LOCAL5
>     -    {"local5",  LOG_LOCAL5},
>     -#endif
>     -#ifdef LOG_LOCAL6
>     -    {"local6",  LOG_LOCAL6},
>     -#endif
>     -#ifdef LOG_LOCAL7
>     -    {"local7",  LOG_LOCAL7},
>     -#endif
>     -    {NULL,      -1},
>     -};
>     -#endif
>     -
>       static const TRANS priorities[] = {
>           {"emerg",   APLOG_EMERG},
>           {"alert",   APLOG_ALERT},
>     @@ -395,29 +331,10 @@ static int open_error_log(server_rec *s,
>
>               s->error_log = dummy;
>           }
>     -
>     -#ifdef HAVE_SYSLOG
>     -    else if (!strncasecmp(s->error_fname, "syslog", 6)) {
>     -        if ((fname = strchr(s->error_fname, ':'))) {
>     -            const TRANS *fac;
>     -
>     -            fname++;
>     -            for (fac = facilities; fac->t_name; fac++) {
>     -                if (!strcasecmp(fname, fac->t_name)) {
>     -                    openlog(ap_server_argv0,
>     LOG_NDELAY|LOG_CONS|LOG_PID,
>     -                            fac->t_val);
>     -                    s->error_log = NULL;
>     -                    return OK;
>     -                }
>     -            }
>     -        }
>     -        else {
>     -            openlog(ap_server_argv0, LOG_NDELAY|LOG_CONS|LOG_PID,
>     LOG_LOCAL7);
>     -        }
>     -
>     +    else if (s->errorlog_provider) {
>     +        s->errorlog_provider_handle = s->errorlog_provider->init(p, s);
>               s->error_log = NULL;
>           }
>     -#endif
>           else {
>               fname = ap_server_root_relative(p, s->error_fname);
>               if (!fname) {
>     @@ -904,7 +821,7 @@ AP_DECLARE(void) ap_register_log_hooks(a
>
>       /*
>        * This is used if no error log format is defined and during startup.
>     - * It automatically omits the timestamp if logging to syslog.
>     + * It automatically omits the timestamp if logging using provider.
>        */
>       static int do_errorlog_default(const ap_errorlog_info *info, char
>     *buf,
>                                      int buflen, int *errstr_start, int
>     *errstr_end,
>     @@ -917,7 +834,7 @@ static int do_errorlog_default(const ap_
>           char scratch[MAX_STRING_LEN];
>       #endif
>
>     -    if (!info->using_syslog && !info->startup) {
>     +    if (!info->using_provider && !info->startup) {
>               buf[len++] = '[';
>               len += log_ctime(info, "u", buf + len, buflen - len);
>               buf[len++] = ']';
>     @@ -1076,22 +993,14 @@ static int do_errorlog_format(apr_array_
>       static void write_logline(char *errstr, apr_size_t len, apr_file_t
>     *logf,
>                                 int level)
>       {
>     -    /* NULL if we are logging to syslog */
>     -    if (logf) {
>     -        /* Truncate for the terminator (as apr_snprintf does) */
>     -        if (len > MAX_STRING_LEN - sizeof(APR_EOL_STR)) {
>     -            len = MAX_STRING_LEN - sizeof(APR_EOL_STR);
>     -        }
>     -        strcpy(errstr + len, APR_EOL_STR);
>     -        apr_file_puts(errstr, logf);
>     -        apr_file_flush(logf);
>     -    }
>     -#ifdef HAVE_SYSLOG
>     -    else {
>     -        syslog(level < LOG_PRIMASK ? level : APLOG_DEBUG, "%.*s",
>     -               (int)len, errstr);
>     -    }
>     -#endif
>     +
>     +    /* Truncate for the terminator (as apr_snprintf does) */
>     +    if (len > MAX_STRING_LEN - sizeof(APR_EOL_STR)) {
>     +        len = MAX_STRING_LEN - sizeof(APR_EOL_STR);
>     +    }
>     +    strcpy(errstr + len, APR_EOL_STR);
>     +    apr_file_puts(errstr, logf);
>     +    apr_file_flush(logf);
>       }
>
>       static void log_error_core(const char *file, int line, int
>     module_index,
>     @@ -1152,7 +1061,7 @@ static void log_error_core(const char *f
>               }
>               else {
>                   /*
>     -             * If we are doing syslog logging, don't log messages
>     that are
>     +             * If we are doing logging using provider, don't log
>     messages that are
>                    * above the module's log level (including a
>     startup/shutdown notice)
>                    */
>                   if (level_and_mask > configured_level) {
>     @@ -1194,7 +1103,7 @@ static void log_error_core(const char *f
>           info.file          = NULL;
>           info.line          = 0;
>           info.status        = 0;
>     -    info.using_syslog  = (logf == NULL);
>     +    info.using_provider= (logf == NULL);
>           info.startup       = ((level & APLOG_STARTUP) == APLOG_STARTUP);
>           info.format        = fmt;
>
>     @@ -1272,7 +1181,14 @@ static void log_error_core(const char *f
>                    */
>                   continue;
>               }
>     -        write_logline(errstr, len, logf, level_and_mask);
>     +
>     +        if (logf) {
>     +            write_logline(errstr, len, logf, level_and_mask);
>     +        }
>     +        else {
>     +            s->errorlog_provider->writer(&info,
>     s->errorlog_provider_handle,
>     +                                         errstr, len);
>     +        }
>
>               if (done) {
>                   /*
>
>
>
>
>
> --
> Born in Roswell... married an alien...
> http://emptyhammock.com/


Mime
View raw message