On Tue, Sep 24, 2013 at 7:32 AM, Jan Kaluža <[email protected]> wrote:
> On 09/23/2013 08:39 PM, Jeff Trawick wrote: > >> On Mon, Sep 23, 2013 at 10:02 AM, <[email protected] >> <mailto:[email protected]>> 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/
