[OpenJDK 2D-Dev] JDK 9: RFR: 8033716: Fix raw and unchecked lint warnings in com.sun.imageio

[Phil Race]

http://cr.openjdk.java.net/~henryjen/jdk9/8033716/1/webrev/src/share/classes/com/sun/imageio/plugins/gif/GIFImageReader.java.sdiff.html

230     public Iterator<ImageTypeSpecifier> getImageTypes(int imageIndex) throws IIOException {

Nitpicky perhaps, but this looks > 80 chars. If so please split it.


-----

W.r.t the following change ...

http://cr.openjdk.java.net/~henryjen/jdk9/8033716/1/webrev/src/share/classes/com/sun/imageio/plugins/jpeg/DHTMarkerSegment.java.sdiff.html


145     class Htable implements Cloneable {

...
<208          protected Object clone() {

>  208         protected Htable clone()

---------

exactly what warning is this suppressing ?

Granted the language now allows returning a sub-class in over-riding, 
but I don't
know why its a problem to return the declared type of the over-ridden 
method?
The tool must have special knowledge of the semantics of the Cloneable 
interface
to even call this out !

Same comment/question applies to DQTMarkerSegment.clone,

JFIFMarkerSegment.clone() and any other like that you might have changed ..
inc. the superclassMarkerSegment which I just spotted ... plus
SOFMarkerSegment,S
SOMarkerSegment

Maybe these will be OK but I need an explanation.

             for (Iterator<JFIFExtensionMarkerSegment> iter = extSegments.iterator(); iter.hasNext();) {232             for (Iterator<JFIFExtensionMarkerSegment> iter = extSegments.iterator(); iter.hasNext();) {

635             for (Iterator<JFIFExtensionMarkerSegment> iter = extSegments.iterator(); iter.hasNext();)

More very long lines ...


I see yet more in JPEGMetadata : 663, 573, 688, 698, 710 .. too many more to call out !

Finally, again note this change when approved should go into jdk9/client.

-phil.


On 2/14/2014 5:05 PM, Henry Jen wrote:
> As there is no other comments so far, I posted updated version adapted 
> comments from Phil.
>
> - removed public field comments from BMPMetadata
> - removed not needed comments from GIFImageMetadata
>
> http://cr.openjdk.java.net/~henryjen/jdk9/8033716/1/webrev/
>
> Cheers,
> Henry
>
>
> On 02/07/2014 04:27 PM, Henry Jen wrote:
>> On 02/07/2014 03:00 PM, Phil Race wrote:
>>> BMPMetadata.java
>>>
>>>
>>>   94     // Fields from CommentExtension
>>>    95     // List of byte[]
>>>   96     public List<byte[]> comments = null; // new ArrayList();
>>>
>>> hmm .. how did you decide this was correct, other than trusting the
>>> comment?
>>
>> For this one, I took it from the comment, after verified similar types
>> in GIF.
>>
>>>
>>> The thing is I can't actually see where this field is used and I'm
>>> inclined
>>> to think this was a copy/paste from the GIF code.
>>>
>>> It would seem the right thing to do is delete these lines.
>>>
>>
>> It is public, so I don't dare to remove it, although it seems like
>> nothing in com.sun.imageio ever make use of this field.
>>
>> Also, from the BMPMetadataFormat, CommentExtension is a string type, so
>> I am not really sure what's the best to do here.
>>
>> If you think it is safe to remove it, I am more than happy to remove it.
>>
>>
>>> In the case of GIFImageMetadata they are used but you left the comments
>>> saying
>>>
>>> // new ArrayList();
>>>
>>> since it looks like you use the diamond operator now that should
>>> not be completely true. Either remove the comment or, since it
>>> seems it was intended to be informative update it to say
>>> // new ArrayList<>();
>>>
>>
>> Agree, I'll remove those comments.
>>
>>> I will have to look at all the subsequent files later ..
>>>
>>
>> Thanks for reviewing it.
>>
>> Cheers,
>> Henry