On Fri, 14 Feb 2025 21:08:31 GMT, Brian Burkhalter <b...@openjdk.org> wrote:
>> Jeremy has updated the pull request incrementally with one additional commit
>> since the last revision:
>>
>> 8160327: fixing typo so `thumbnailPos` can be zero
>>
>> The `thumbnailLength` cannot be zero, but the position can be.
>
> src/java.desktop/share/classes/com/sun/imageio/plugins/jpeg/ExifMarkerSegment.java
> line 28:
>
>> 26: package com.sun.imageio.plugins.jpeg;
>> 27:
>> 28: import java.io.InputStream;
>
> I suggest putting the imports on lines 28-51 in alphabetical order.
This is updated
> src/java.desktop/share/classes/com/sun/imageio/plugins/jpeg/JPEGImageReader.java
> line 1758:
>
>> 1756: }
>> 1757:
>> 1758: // Now we know that there is a jfif segment and that iis
>> is good
>
> I think `iis` should be `it` in this comment.
This comment no longer exists after other revisions
> src/java.desktop/share/classes/com/sun/imageio/plugins/jpeg/JPEGMetadata.java
> line 266:
>
>> 264: && (buf[ptr+7] == 0)
>> 265: && (buf[ptr+8] == 0)) {
>> 266: newGuy = new ExifMarkerSegment(buffer);
>
> The `IllegalArgumentException` from the `ExifMarkerSegment` constructor
> should be handled somehow. Perhaps just fall back to setting `newGuy` to a
> plain `MarkerSegment`?
This is updated (see this thread
https://github.com/openjdk/jdk/pull/22898#discussion_r1955194737 )
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/22898#discussion_r1967118843
PR Review Comment: https://git.openjdk.org/jdk/pull/22898#discussion_r1967118674
PR Review Comment: https://git.openjdk.org/jdk/pull/22898#discussion_r1967118950
