Hi Sebastian,
I will help to push the patch, if people all agreed the changes proposed.
I pulled your patch and generated the webrev at
http://cr.openjdk.java.net/~sherman/7084245/webrev
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
one for
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
previously
These changes might go different path to get it, if agreed.
It appears Xuelei.Fan from security might have some options on the
changes made
in security area. So Xuelei, do you want me to pull out the changes in
security
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
cause of
this InternalError. But for the case of InternalError(msg) only, the
proposed change
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
InternalError. As
suggested in the Throwable API doc, the "hiding" might be purposely, to
separate
different levels of abstraction when throw the InternalError.
-Sherman
On 08/28/2011 12:35 PM, Sebastian Sickelmann wrote:
Hi, here is a webrev[1] for some cleanup that i want to integrated in
tl-repositories.
Alan Bateman had scanned the changes and gave me some good input[3]
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[2].
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[1] 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
[1] http://oss-patches.24.eu/openjdk8/InternalError/part2/7084245_main_0/
[2]
http://oss-patches.24.eu/openjdk8/InternalError/part2/7084245_concurrent_0/
[3]
http://mail.openjdk.java.net/pipermail/core-libs-dev/2011-August/007563.html
