tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Christopher Schultz <ch...@christopherschultz.net>
Subject Re: svn commit: r1523685 - in /tomcat/tc7.0.x/trunk: ./ java/org/apache/juli/ClassLoaderLogManager.java webapps/docs/changelog.xml
Date Tue, 17 Sep 2013 14:43:18 GMT
Konstantin,

On 9/16/13 5:31 PM, Konstantin Kolinko wrote:
> 2013/9/16  <schultz@apache.org>:
>> Author: schultz
>> Date: Mon Sep 16 14:49:26 2013
>> New Revision: 1523685
>>
>> URL: http://svn.apache.org/r1523685
>> Log:
>> Added logging of logging.properties location when system property is set.
>>
>> Modified:
>>     tomcat/tc7.0.x/trunk/   (props changed)
>>     tomcat/tc7.0.x/trunk/java/org/apache/juli/ClassLoaderLogManager.java
>>     tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml
>>
> 
> 1. Code style.

Sorry 'bout that. I'll fix it.

> 2. Both branches of 'if' have duplicate code that reads the property.
> It could be moved up a bit.
> 
> (I am OK with re-reading the property on each invocation of this
> method, as I think one can change it at runtime,  or may configure it
> in catalina.properties)

This code runs so infrequently I didn't think it mattered. As for
reading it once in each branch, I had originally only been logging (to
stderr) if the logger was non-null. IMO, the code I have is no slower
than if it had been written like this:

 boolean debugLog = Boolean.getProperty(DEBUG_PROPERTY);

 if(null)
   if(debugLog)
 else
   if(debugLog)

...except that whats in svn has fewer lines of code. Let me know if you
feel strongly that this should be changed.

> 3. You have to update catalina.policy. Otherwise it will fail when
> running with a SecurityManager.

Thanks for pointing that out: I had completely forgotten about the
SecurityManager. I'll get that fixed, too.

-chris


Mime
View raw message