<i18n dev> Request for review: 7084245: Update usages of InternalError to use exception chaining
xueming.shen at oracle.com
Mon Aug 29 12:10:18 PDT 2011
I will help to push the patch, if people all agreed the changes proposed.
I pulled your patch and generated the webrev at
with couple changes from your original patch.
(1) Undo the changes in DecimalFormat.java and Format.java.
while arguably your suggested change might be the correct/better
the situation, but it's a behavior change, personally I don't
think you want
to change the behavior, whether it is a "better" solution (not
just look better)
or "bug fix", in this kind of clean-up project. Leave that to
separate "bug fix"
patch, if you believe the existing code is buggy/wrong.
(2) Removed your comment and left the printStackTrace() in LoopPipe.java
untouched. Again, that printStackTrace() might be unintentional,
a leftover of
the debug version for example, but I'm not sure. Until the 2d
engineer that is
the case, I would prefer to leave it un-touched.
(3) Removed the concurrent package changes from the list. As discussed
These changes might go different path to get it, if agreed.
It appears Xuelei.Fan from security might have some options on the
in security area. So Xuelei, do you want me to pull out the changes in
area and leave that for your to deal with separately?
Personally I think it's nice to make the switch to the new chaining
version if the
original code is "new InternalError(msg) + initCause()", in which case
it has the
clear intention that implementation does want to expose the original
this InternalError. But for the case of InternalError(msg) only, the
actually again changes the behavior/intention of the original code, in
which it might
not be desirable to expose the original cause when throw the
suggested in the Throwable API doc, the "hiding" might be purposely, to
different levels of abstraction when throw the InternalError.
On 08/28/2011 12:35 PM, Sebastian Sickelmann wrote:
> Hi, here is a webrev for some cleanup that i want to integrated in
> Alan Bateman had scanned the changes and gave me some good input
> for further discussion here:
> The changes to java.util.concurrent should go through Doug Lea's
> upstream CVS. Alan told me that Chris Hegarty is on this topic
> already. The suggested changes for this is here.
> I have changed some classes in awt / sun.java2d maybe someone of the
> 2d-dev maillinglist can look at these changes.
> I also changed some classes in java/securtiy maybe someone of
> security-dev maillinglist can look at these changes.
> Let me know if there is a need to split/rebase the main-webrev to
> review and/or push it individually.
> Mostly the patch changes exception-chains. But there are some places
> where the patch changes behavoir:
> - I removed some printstackTraces in
> sun.java2d.pipe.LoopPipe.getStrokesSpans and sun.misc.Launcher (Alan
> told me that kumar maybe want to have a look at it?).
> - I changed java.text.Format.clone not to return null. I think it will
> never happen. But if so throwing an InternalError seems to be better
> than returning null and let all the extended classes crash in there
> clone Method with a NullPointerException. And so catching an Exception
> in java.text.DecimalFormat.clone is unnecessary.
> -- Sebastian
>  http://oss-patches.24.eu/openjdk8/InternalError/part2/7084245_main_0/
More information about the i18n-dev