On Mon, Sep 23, 2013 at 10:02 AM, <jkal...@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. 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 > 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/