Hi Jay,

Sorry but I have a few picky comments.

 377         // If imagePositions list doesn't contain any of the image stream
 378         // starting position(i.e tables-only image) we should not try to 
access
 379         // imagePositions.size() as it done below, because it will lead to
 380         // IndexOutOfBoundsException with index -1.

You might consider this verbiage instead:

If the image positions list is empty as in the case of a tables-only stream, 
then attempting to access the element at index
imagePositions.size() - 1 will cause an IndexOutOfBoundsException.

 381         if (seekForwardOnly && (!(imagePositions.isEmpty()))) {

I think this is more readable if some parentheses are eliminated:

 381         if (seekForwardOnly && !imagePositions.isEmpty()) {

 499         // We should not try to read image information from an input stream
 500         // which only contains tables-only(StreamMetadata) information.

You might consider this verbiage instead:

If the image positions list is empty as in the case of a tables-only stream, 
then no image data can be read.

No need to update the webrev.

Thanks,

Brian

On Jan 7, 2018, at 11:23 PM, Jayathirth D V <jayathirth....@oracle.com> wrote:

> I have removed the usage of tablesOnlyStream variable in code and updated the 
> comment section.
> Please find updated webrev for review:
> http://cr.openjdk.java.net/~jdv/8191073/webrev.01/

Reply via email to