On 02.06.2010 22:42, Stefan Fritsch wrote:
I have updated my patch to the latest trunk and to use the variant
where the module for logging is selected with APLOG_USE_MODULE and
AP_DECLARE_MODULE macros:
...
Multi-file modules have to use APLOG_USE_MODULE in the other
files.
The patch is at
http://people.apache.org/~sf/per-module-loglevel-v4/ ,
both as one file and as a series split into more or less logical
chunks. Comments are very welcome. Also, it would be nice if
someone could try it with a different compiler than gcc.
Some comments based on visual inspection. Next step (building/testing)
is on my ToDo list. No major findings, only minor stuff.
0002-Introduce-log-levels-trace1-.-trace8.patch
-----------------------------------------------
Minor: Use of LOG_PRIMASK and APLOG_LEVELMASK: LOG_PRIMASK usually comes
from sys/syslog.h and we set it to "7" if it is not defined. Previously
both masks were the same. Since this is no longer the case, I wouldn't
-#define APLOG_LEVELMASK 7 /* mask off the level value */
+#define APLOG_LEVELMASK ((LOG_PRIMASK << 1) + 1) /* mask off the level
value */
but instead keep our mask at a fixed value:
+#define APLOG_LEVELMASK 15 /* mask off the level value */
and then when using syslog
- syslog(level_and_mask, "%s", errstr);
+ syslog(level_and_mask < APLOG_DEBUG ? level_and_mask : APLOG_DEBUG,
+ "%s", errstr);
instead
+ syslog(level_and_mask < LOG_PRIMASK ? level_and_mask : APLOG_DEBUG,
+ "%s", errstr);
0005-introduce-per-module-log-levels.patch
------------------------------------------
ap_get_module_loglevel() and ap_set_module_loglevel() get the module
index as an argument, but the doc says "get at *their own*
module-specific loglevel". It seems you can get/set the levels for all
modules, not only your own. But well... If you change the wording,
there's an anlogous comment in
0009-add-loglevel-to-request_rec-and-conn_rec.patch.
Identation in the line: "Setting LogLevel for all modules to %s", arg);
(the line is also contained in
0010-Introcduce-per-dir-loglevel-configuration.patch, but unchanged there)
Twice outdated code comments in log_error_core: "* above the server log
level".
0013-Adjust-loglevels-to-log-less-at-levels-INFO-and-DEBU.patch
---------------------------------------------------------------
Indentation: "proxy: HTTP: canonicalising URL %s", url);
Indentation: loglevel = APLOG_INFO;
Indentation in the "new if" block in server/protocol.c
Indentation in modules/ssl/ssl_engine_kernel.c whenever ap_log_error is
replaced by ap_log_cerror (argument indentation)
0017-associate-CONNECT-errors-with-the-request.patch
----------------------------------------------------
Indentation in modules/proxy/mod_proxy.h whenever ap_log_error is
replaced by ap_log_rerror (argument indentation)
Additional remarks
------------------
We need to fix and add some docs
- removed directives for mod_ssl, mod_rewrite and mod_logio
- new syntax for LogLevel
- merging and inheritance of LogLevels between servers and modules
Later possible improvement: should we add the module_name to the log
line? Since you already know the index, associating the name would be
easy. The question is, whether the info is important enough to add to
the log line.
Regards,
Rainer
