On 14.04.2015 06:54, Kaspar Brand wrote:
> On 13.04.2015 22:05, Ruediger Pluem wrote:
>> Shouldn't we only do that in case that vit->log.level is set to
>> main_server->log.level?
>> Don't we lose the configuration done by the user for this particular host
>> otherwise?
>
> Yes, you're right - thanks for the review. It also noticed this in the
> meantime, and when I tried to come up with a fix, more special cases
> were turning up (whether main_server->log.level is still at
> DEFAULT_LOGLEVEL might also have to be taken into account).
>
> Hopefully I'll find time for more closely examining this tomorrow.
> Otherwise I will revert r1672014, as it is clearly not a complete
> solution yet.
Reverted r1672014. After more digging, I think there's no practical way
to base the decision whether to reset on the current value of
virt->log.level when reaching ap_fixup_virtual_hosts - too many
conceivable combinations of global LogLevel vs. per-vhost LogLevel (and
the order they appear in the config). Extending the ap_logconf struct
seems unavoidable if the approach with adjustments in
ap_init_virtual_host/ap_fixup_virtual_hosts is the right one.
Another idea would be to apply the default log level in log_error_core()
when we encounter APLOG_UNSET, as illustrated by the attached patch.
Maybe doing this check in each log_error_core() invocation adds too much
overhead, given that it's actually just addressing an issue which occurs
at startup/reload?
The other option to fix the problem which originally triggered this
investigation would be to simply replace "cmd->server" in
ssl_engine_config.c:836 with "NULL", though it would only fix this
particular case, and not help with other places with the same underlying
problem.
Kaspar
Index: include/http_log.h
===================================================================
--- include/http_log.h (revision 1673835)
+++ include/http_log.h (working copy)
@@ -128,6 +128,8 @@ extern "C" {
*/
#define APLOG_NO_MODULE -1
+#define APLOG_UNSET (APLOG_NO_MODULE - 1)
+
#ifdef __cplusplus
/**
* C++ modules must invoke ::APLOG_USE_MODULE or ::AP_DECLARE_MODULE in
Index: server/config.c
===================================================================
--- server/config.c (revision 1673838)
+++ server/config.c (working copy)
@@ -52,7 +52,6 @@
#include "util_varbuf.h"
#include "mpm_common.h"
-#define APLOG_UNSET (APLOG_NO_MODULE - 1)
/* we know core's module_index is 0 */
#undef APLOG_MODULE_INDEX
#define APLOG_MODULE_INDEX AP_CORE_MODULE_INDEX
Index: server/log.c
===================================================================
--- server/log.c (revision 1673835)
+++ server/log.c (working copy)
@@ -1075,6 +1075,11 @@ static void log_error_core(const char *file, int l
int configured_level = r ? ap_get_request_module_loglevel(r,
module_index) :
c ? ap_get_conn_server_module_loglevel(c, s,
module_index) :
ap_get_server_module_loglevel(s,
module_index);
+
+ /* Temporarily adjust to the default if the level hasn't been set yet
*/
+ if (configured_level == APLOG_UNSET)
+ configured_level = ap_default_loglevel;
+
/*
* If we are doing normal logging, don't log messages that are
* above the module's log level unless it is a startup/shutdown notice
