RFR: 8059767: FileHandler should allow 'long' limits and handle overflow of MeteredStream.written.

[Daniel Fuchs]

Hi,

Following a feedback from Alan - I dropped 'SecurityException'
from the throws clause of the new constructor - keeping
only the @throws in the API documentation.

Here is the new webrev (the above is the only change compared
to the previous webrev.01).

http://cr.openjdk.java.net/~dfuchs/webrev_8059767/webrev.02

I also logged JDK-8060457 to follow up and cleanup that in
all handlers subclasses (no impact on the actual
implementation / behavior - it's just cleanup).
Attempting the cleanup in this fix was just too much noise.

best regards,

-- daniel


On 08/10/14 11:46, Daniel Fuchs wrote:
> On 08/10/14 08:26, Ivan Gerasimov wrote:
>> Hi Daniel!
>>
>> Would it make sense to make the old constructor FileHandler(String,
>> *int*, int, boolean) call the new one, casting the limit to long?
>
> Hi Ivan,
>
> I was about to reply 'no' out of consistency with the other
> constructors - which don't call each others - when I realized
> that it was not be possible to make the other constructors
> call each others anyway - as passing a parameter to a constructor
> means overriding the default value read from the configuration
> file.
>
> This however doesn't hold in our case as both constructors
> have exactly the same code - so there's no reason for not making
> FileHandler(String, *int*, int, boolean) call
> FileHandler(String, *long*, int, boolean).
>
> Which now makes me question whether I should add a second constructor
> FileHandler(String, *long*, int) to shadow FileHandler(String, *int*,
> int) - as Stanimir aleready suggested.
>
> My feeling is that the case where you would want to rely on the
> default value of 'append' configured in the logging.properties file
> instead of specifying one to the constructor should be sufficiently
> rare to not warrant the addition of a second constructor.
> And if you really wanted to rely on that default value you could
> go and fetch it before calling the constructor.
>
> Anyway - good remark - and here is a new webrev including yours and
> Jason's comments.
>
> http://cr.openjdk.java.net/~dfuchs/webrev_8059767/webrev.01
>
> best regards,
>
> -- daniel
>
>>
>> Sincerely yours,
>> Ivan
>>
>> On 07.10.2014 17:13, Daniel Fuchs wrote:
>>> Hi,
>>>
>>> Please find below a patch for:
>>>
>>> 8059767: FileHandler should allow 'long' limits and handle overflow
>>>          of MeteredStream.written.
>>> https://bugs.openjdk.java.net/browse/JDK-8059767
>>>
>>> webrev:
>>> http://cr.openjdk.java.net/~dfuchs/webrev_8059767/webrev.00/
>>>
>>> This follows an issue reported on this list:
>>> http://mail.openjdk.java.net/pipermail/core-libs-dev/2014-August/028280.html
>>>
>>>
>>>
>>> The patch changes 'limit' from 'int' to 'long', fixes the
>>> handling of overflow, and adds a new constructor that allows
>>> to pass a long for 'limit'.
>>> It also makes it possible to specify a long value for 'limit'
>>> in the configuration.
>>>
>>> The test checks the handling of overflow by tweaking the
>>> internal of MeteredStream through reflection. Not ideal, but
>>> I couldn't find any other way.
>>>
>>> best regards,
>>>
>>> -- daniel
>>>
>>>
>>
>