Konstantin Kolinko wrote: > 1. There is a bug, that sneaked in rev.781753 > http://svn.apache.org/viewvc?rev=781753&view=rev > That is: > the timeZoneNoDST field is never assigned and remains null > It was ok in 5.5.27.
That is an error in my backport. That line should never have been deleted. I'll fix that now. > 2. struct.currentDateString is calculated in two different places > I propose to encapsulate that code into AccessDateStruct > > 3. Invocation of getDate() in invoke() (the only place where getDate() > is called): > I think that getDate() also can be incapsulated into AccessDateStruct. > Actually, I think of the following: > 1) In invoke() there is already a variable that stores current time > millis: "t2" > I propose to replace that getDate() with > AccessDateStruct.setDate(t2) that will perform the same job. > 2) Change the signature of replace(..., Date, ..) method, and pass a > reference to the AccessDateStruct instead of a Date. > In the future the replace(..) methods of this class can be changed to > accept a StringBuffer, instead of returning a String, but that can be > another story. > > If AccessDateStruct is passed into the replace() method, > timeTakenFormatter can be added to it as well. Maybe. > > 4. The log() method still creates a new Date() once in a second. > The Date instance can be stored in some member variable and reused. It > is in a synchronized block, so it will be safe. > I do not like, that it is the same once-in-a-second synchronized > block, that we already avoided once by using those ThreadLocals. > Can be addressed separately, though. Thanks for this. I'll see about updating the patch. > +1 to the patch regarding AccessLogValve. I'll write about two other > classes later, if I find anything. (By the first glance, I have not > noticed any problems there). > > Best regards, > Konstantin Kolinko > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org > For additional commands, e-mail: dev-h...@tomcat.apache.org > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
