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




Reply via email to