httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jeff Trawick <traw...@gmail.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:39:46 GMT
On Tue, Sep 24, 2013 at 7:32 AM, Jan Kalu┼ża <jkaluza@redhat.com> wrote:

> 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.
>

Thank you very much!


>
> 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<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<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<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<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<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<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<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/
>>
>
>


-- 
Born in Roswell... married an alien...
http://emptyhammock.com/

Mime
View raw message