[OpenJDK 2D-Dev] Request for review: 7196571, 7196572, 7196573: javac warnings cleanup from Adopt OpenJDK bugathon

Phil Race philip.race at oracle.com
Tue Sep 11 20:07:51 UTC 2012

I reviewed all the changes and they look fine to me with just formatting 
and one code change question or suggestion

-* an identation issue at line 596 here


The removal of the cast means the other lines aren't aligned any more

* I also wonder why lines 837 & 838 can't be collapsed into one line ?
0x00000001 << (31 - bOffset % 32));

* By contrast lines 1035 and 1115 here are now much, much > 80 chars

Also for both of these cases, you added .asSubClass(..).
Now when I look at the new code there, it looks a lot busier, all to 
avoid one
simple (ImageWriterSpi) cast. Is it really worth it ?
And if the change is made, maybe you should check for ClassCastException.
I guess we already vulnerable to that, although its rather unlikely to 
occur ...

I double checked for most of the serial verson uids in the imaging & 
printing classes
that they were generated from running serialver on the classes so as to 
the existent values, so that looks correct.

Don't forget you need a second reviewer for all the changes.


On 9/11/2012 11:40 AM, Martijn Verburg wrote:
> Hi all,
> Artem Ananiev very kindly raised bugs and a webrev for the patches
> sent in from a Bugathon we ran back in April (patches have been tested
> against latest source tree).
> The bug numbers are:
> 7196571: [Bugathon] Reduce the number of javac warnings in ImageIO
> 7196572: [Bugathon] Reduce the number of javac warnings in color management
> 7196573: [Bugathon] Reduce the number of javac warnings in imaging
> The corresponding webrevs are at:
> http://cr.openjdk.java.net/~art/Bugathon-2012/webrev.imageio/
> http://cr.openjdk.java.net/~art/Bugathon-2012/webrev.j2d-color/
> http://cr.openjdk.java.net/~art/Bugathon-2012/webrev.j2d-image/
> http://cr.openjdk.java.net/~art/Bugathon-2012/webrev.j2d-print/
> Thanks to Artem, Stuart and Phil for helping me navigate through the
> AWT/2D waters :-)
> Cheers,
> Martijn

More information about the 2d-dev mailing list