logging-log4j-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ceki Gülcü <c...@qos.ch>
Subject Re: Dueling commits on CachedDateFormat
Date Sun, 26 Dec 2004 19:52:37 GMT
At 05:06 PM 12/26/2004, Curt Arnold wrote:

>On Dec 26, 2004, at 8:59 AM, Ceki Gülcü wrote:
>
>Just single 'S' and 'SS' hadn't been a problem for a for the last previous 
>iterations, it was only when you tacked on trailing '0' that the pattern 
>finder could get fooled, though I didn't make that distinction in some of 
>my messages.  Now it also checks for length changes, if the formatted 
>strings length changes, then findMillisecondStart should return 
>UNRECOGNIZED_MILLISECONDS and only 1 ms caching should be effective.

As you say, findMillisecondStart method in previous iterations would also 
return UNRECOGNIZED_MILLISECONDS for single or double S. The only reason I 
tried 'SS0' was because I thought two 'S' meant the most significant two 
digits of the millisecond field (or units of 10 milliseconds) and single 
'S' meant the most significant digit of the millisecond field (or units of 
100 milliseconds). Instead, the number of 'S' only influences padding. 
Thus, the 'SS0' format in the config file does not necessarily indicate 
hostility. It could also be ignorance as was previously the case for me.

>>It looks like you are no longer taking advantage of the case where there 
>>is no millisecond field to print, in which case millisecond formatting 
>>and findMillisecondStart calculations can be skipped.
>
>Definitely not the intention.  CachedDateFormatTest.test13 checks that 
>findMillisecondStart returns NO_MILLISECONDS for a date format.
>Reviewing the code in CachedDateFormat.format looks like it should do the 
>right thing when millisecondStart == NO_MILLISECONDS, but I haven't single 
>stepped through it.

findMillisecondStart  returns NO_MILLISECONDS but that knowledge does not 
seem to be used in the format() method.

As I wrote in a previous message, instead of invoking findMillisecondStart 
every second, we could use the knowledge of the pattern to avoid using 
CachedDateFormat for unsafe methods. Version 15 invokes 
findMillisecondStart  about once a second which not consequential but also 
also quite unnecessary. I feel that version 15 constitutes a regression wrt 
to version 1.12. Nevertheless, in reverence for your hard work and 
initiative, I am happy to defer to your judgement.

>I do feel strongly about it.  Those checks add trivial complexity, I don't 
>think that two instances of:
>
>if (slotBegin > time) {
>     slotBegin -= 1000;
>}
>
>add a huge maintenance requirement and they protect the rest of the code 
>that behaves poorly if the number of milliseconds goes negative.
>Unlike the 'SS0' format tricks which require the configurator to be 
>hostile, you could get pre-1970 dates by reading XML files or from a 
>hostile SocketAppender which might be out of your control in addition to 
>setting the system time back.

I was not only thinking of the if (slotBegin > time) { slotBegin -= 1000; } 
check. There are other places that assume slotBegin to only take positive 
values. For example, set setTimeZone method. As mentioned previously, I am 
happy to defer to your judgement.


-- 
Ceki Gülcü

   The complete log4j manual: http://qos.ch/log4j/



---------------------------------------------------------------------
To unsubscribe, e-mail: log4j-dev-unsubscribe@logging.apache.org
For additional commands, e-mail: log4j-dev-help@logging.apache.org


Mime
View raw message