tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Christopher Schultz <ch...@christopherschultz.net>
Subject Re: svn commit: r1579941 - /tomcat/tc6.0.x/trunk/STATUS.txt
Date Fri, 21 Mar 2014 18:07:13 GMT
Konstantin,

On 3/21/14, 10:44 AM, Konstantin Kolinko wrote:
> 2014-03-21 17:46 GMT+04:00  <schultz@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


Mime
View raw message