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 <[email protected]> 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/