Konstantin, On 3/21/14, 10:44 AM, Konstantin Kolinko wrote: > 2014-03-21 17:46 GMT+04:00 <schu...@apache.org>: >> Author: schultz >> Date: Fri Mar 21 13:46:53 2014 >> New Revision: 1579941 >> >> URL: http://svn.apache.org/r1579941 >> Log: >> Updated votes. >> >> Modified: >> tomcat/tc6.0.x/trunk/STATUS.txt >> >> Modified: tomcat/tc6.0.x/trunk/STATUS.txt >> URL: >> http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/STATUS.txt?rev=1579941&r1=1579940&r2=1579941&view=diff >> ============================================================================== >> --- tomcat/tc6.0.x/trunk/STATUS.txt (original) >> +++ tomcat/tc6.0.x/trunk/STATUS.txt Fri Mar 21 13:46:53 2014 >> @@ -36,45 +36,22 @@ PATCHES PROPOSED TO BACKPORT: >> +1: markt, kkolinko >> -1: schultz: The idea of the patch is fine: I'm actually +1. >> I have some small nits: >> - 1. DocumentBuilderFactory is not thread-safe, and shouldn't >> - be shared. 2. Two instances of swallowing IOException >> + 2. Two instances of swallowing IOException >> when closing File streams. We should at least log a warning. >> - It looks like there is an opportinity to use StringBuilder >> - instead of StringBuffer, there, too, if you want. >> + The case of InputStream vs OutputStream is not relevant: >> + a stream left open should be logged. Honestly, it will >> + pretty much never happen, but that's no excuse not to log >> + a potential problem. > > > 1) Is your vote still -1, > or -0, or +0?
Still -1, just like STATUS.txt still says. > 2) If inputStream.close() fails it does not mean that the stream is left open. > Also no data is lost (unlike outputStream). It doesn't mean that the stream is definitely left open. But the stream *could* be left open. > Anyway it cannot be a warning. It can be a debug message at best. Why not a WARNING? -chris
signature.asc
Description: OpenPGP digital signature
