Thanks for the review Jason!
On 24/03/15 18:01, Jason Mehrens wrote:
Hi Daniel,
Looks good. The only other alternative would be to use
java.io.CharConversionException over IOException. We could even consider
dropping the cause because the subclass of I/O exception would convey the same
meaning.
Here is a an updated webrev where I use
java.io.CharConversionException instead of plain IOException.
http://cr.openjdk.java.net/~dfuchs/webrev_8075810/webrev.01/
Note: I had a look in the JDK sources and CharConversionException
doesn't appear to be widely used. Is this the better alternative?
I'd be happy with both (webrev.01 or webrev.00).
Using CharConversionException means that we have to trust that
Properties.load will obey its spec and only throw IAE in case
of character conversion issues.
Minor formatting issues with a missing space after the catch keyword
Done. Thanks for catching it.
and missing a tab ahead of 'props.load'
That is an artifact of how webrev calls diff I think.
If you look at the frames version you will see that the tab is here.
best regards,
-- daniel
Jason
----------------------------------------
Date: Tue, 24 Mar 2015 16:06:47 +0100
From: daniel.fu...@oracle.com
To: lance.ander...@oracle.com
Subject: Re: RFR: 8075810: LogManager.readConfiguration may throw undocumented
IllegalArgumentException
CC: core-libs-dev@openjdk.java.net
Hi Lance,
Thanks for the review!
On 24/03/15 16:01, Lance Andersen wrote:
Hi Daniel,
This looks OK but I might suggest clarifying that the cause could be set
in the javadoc. Now that being said, I am not sure how consistent we
are across the JDK or just make the assumption people will check the
cause if they desire.
Hmmm. I have the feeling that the best place for that would be i the
release notes - rather than in the API doc (which reminds me I should
plan to add some release note text to the issue).
As far as backporting, unless it really needed, I would probably would
not as a change such as this typically should be done as part of an
errata/MR (due to change of behavior and it is not that big of an issue).
Right, my thinking too. Thanks for sharing your opinion!
best regards,
-- daniel
Best
Lance
On Mar 24, 2015, at 10:42 AM, Daniel Fuchs <daniel.fu...@oracle.com
<mailto:daniel.fu...@oracle.com>> wrote:
Hi,
Please find below a fix for
8075810: LogManager.readConfiguration may throw
undocumented IllegalArgumentException
https://bugs.openjdk.java.net/browse/JDK-8075810
The proposed fix is to catch the IllegalArgumentException and
wrap it in an IOException, since LogManager.readConfiguration
is specified to throw IOException "if there are problems reading
from the stream."
http://cr.openjdk.java.net/~dfuchs/webrev_8075810/webrev.00/
Initial discussion around the issue can be seen here:
http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-March/032348.html
Question for reviewers: I will log a CCC for this - but I am wondering
whether I should plan to backport the fix to earlier release?
best regards,
-- daniel
<http://oracle.com/us/design/oracle-email-sig-198324.gif>
<http://oracle.com/us/design/oracle-email-sig-198324.gif><http://oracle.com/us/design/oracle-email-sig-198324.gif>
<http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen|
Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
lance.ander...@oracle.com <mailto:lance.ander...@oracle.com>
