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: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
