On Dec 26, 2004, at 8:59 AM, Ceki G�lc� wrote:
I've added tests to check all the previously misinterpreted patterns.
The previous tricks of using "SS0" and were actually easier to catch using the length of the string changes
Although it was used in my earlier tests, "SS0" is not a very plausible format string, double and especially a single 'S' are more plausible. (Single 'S' means "print the millisecond field 'as is', with no padding").
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.
between the evaluation at the integral second and the magic millisecond counts. I was going to say that I couldn't think of a trick to play on the caching at this time, but then I thought of one and will commit a fix in just a second.
Hopefully, I've cut down the creation of Date's even more than your last iteration. I haven't single stepped through the code or ran any performance tests. If I missed something simple, please feel free to tweak it, but let's confer before you do any major surgery.
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.
I noticed that you had removed checks for pre-1970 dates. I had a test that was should have failed but didn't which I have now repaired.
Obviously it should be rare that anyone sets their system time back 35 years, but I'd like the cache to be able to handle if somebody uses it in a different context.
There are many other places where we assume that the environment in which log4j runs, has time set to a date later than 1970. Pretending that we support times earlier than 1970 makes the code harder to understand and to maintain. Unless you feel strongly about it, my preference is to have the pre-1970 checks removed.
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.
--------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
