Re: [OpenJDK 2D-Dev] [11] RFR: [JDK-4954348]: JPGWriter.getNumThumbnailsSupported does not return -1 when passing null values
+1 -phil On 03/27/2018 04:41 AM, Prahalad Kumar Narayanan wrote: Hello Phil, Sergey and Jay Thank you for your time in review. I've incorporated your review suggestions. In the updated code, ImageWriter gets disposed appropriately. Kindly review the updated code at your convenience. Link: http://cr.openjdk.java.net/~pnarayanan/4954348/webrev.01/ Thank you once again Have a good day Prahalad N. -Original Message- From: Philip Race Sent: Tuesday, March 27, 2018 2:40 AM To: Sergey Bylokhov Cc: Prahalad Kumar Narayanan; 2d-dev Subject: Re: [OpenJDK 2D-Dev] [11] RFR: [JDK-4954348]: JPGWriter.getNumThumbnailsSupported does not return -1 when passing null values On 3/23/18, 11:56 AM, Sergey Bylokhov wrote: Hi, Prahalad. A few small comments about the test: - Is it possible to test all installed ImageWriterSpi? It seems that the test itself is not a JPEG plugin specific? None of the other plugins support thumbnails so they will all return 0 regardless. So I think that aspect of the test is OK - You will need to dispose the jpgWriter even in case of exception. Probably should fix that. Everything else seems fine. -phil. On 23/03/2018 01:10, Prahalad Kumar Narayanan wrote: Kindly review the changes at your convenience Link: http://cr.openjdk.java.net/~pnarayanan/4954348/webrev.00 Thank you Have a good day Prahalad N.
Re: [OpenJDK 2D-Dev] [11] RFR: [JDK-4954348]: JPGWriter.getNumThumbnailsSupported does not return -1 when passing null values
Looks fine. On 27/03/2018 04:41, Prahalad Kumar Narayanan wrote: Hello Phil, Sergey and Jay Thank you for your time in review. I've incorporated your review suggestions. In the updated code, ImageWriter gets disposed appropriately. Kindly review the updated code at your convenience. Link: http://cr.openjdk.java.net/~pnarayanan/4954348/webrev.01/ Thank you once again Have a good day Prahalad N. -Original Message- From: Philip Race Sent: Tuesday, March 27, 2018 2:40 AM To: Sergey Bylokhov Cc: Prahalad Kumar Narayanan; 2d-dev Subject: Re: [OpenJDK 2D-Dev] [11] RFR: [JDK-4954348]: JPGWriter.getNumThumbnailsSupported does not return -1 when passing null values On 3/23/18, 11:56 AM, Sergey Bylokhov wrote: Hi, Prahalad. A few small comments about the test: - Is it possible to test all installed ImageWriterSpi? It seems that the test itself is not a JPEG plugin specific? None of the other plugins support thumbnails so they will all return 0 regardless. So I think that aspect of the test is OK - You will need to dispose the jpgWriter even in case of exception. Probably should fix that. Everything else seems fine. -phil. On 23/03/2018 01:10, Prahalad Kumar Narayanan wrote: Kindly review the changes at your convenience Link: http://cr.openjdk.java.net/~pnarayanan/4954348/webrev.00 Thank you Have a good day Prahalad N. -- Best regards, Sergey.
Re: [OpenJDK 2D-Dev] [11] RFR: [JDK-4954348]: JPGWriter.getNumThumbnailsSupported does not return -1 when passing null values
Hello Phil, Sergey and Jay Thank you for your time in review. I've incorporated your review suggestions. In the updated code, ImageWriter gets disposed appropriately. Kindly review the updated code at your convenience. Link: http://cr.openjdk.java.net/~pnarayanan/4954348/webrev.01/ Thank you once again Have a good day Prahalad N. -Original Message- From: Philip Race Sent: Tuesday, March 27, 2018 2:40 AM To: Sergey Bylokhov Cc: Prahalad Kumar Narayanan; 2d-dev Subject: Re: [OpenJDK 2D-Dev] [11] RFR: [JDK-4954348]: JPGWriter.getNumThumbnailsSupported does not return -1 when passing null values On 3/23/18, 11:56 AM, Sergey Bylokhov wrote: > Hi, Prahalad. > A few small comments about the test: > - Is it possible to test all installed ImageWriterSpi? It seems that > the test itself is not a JPEG plugin specific? None of the other plugins support thumbnails so they will all return 0 regardless. So I think that aspect of the test is OK > - You will need to dispose the jpgWriter even in case of exception. Probably should fix that. Everything else seems fine. -phil. > > On 23/03/2018 01:10, Prahalad Kumar Narayanan wrote: >> Kindly review the changes at your convenience >> Link: http://cr.openjdk.java.net/~pnarayanan/4954348/webrev.00 >> >> Thank you >> Have a good day >> >> Prahalad N. >> > >
Re: [OpenJDK 2D-Dev] [11] RFR: [JDK-4954348]: JPGWriter.getNumThumbnailsSupported does not return -1 when passing null values
Hi Phil, > Test case works properly before and after the code change. ? You mean it does not fail before the fix ? It definitely fails for me before the fix. - I mean as intended Phil. It fails before the fix and after the fix it passes. > Since we are touching getNumThumbnailsSupported() function and it is only > overridden in JPEGImageWriter we can add override annotation for the same. I would be more inclined to address that in a separate bug that adds the annotation for all the over-ridden JPEG reader+writer methods, rather than adding just one in the file. - Sure. We can take that in separate bug. Thanks, Jay -Original Message- From: Philip Race Sent: Tuesday, March 27, 2018 2:39 AM To: Jayathirth D V Cc: Prahalad Kumar Narayanan; 2d-dev Subject: Re: [OpenJDK 2D-Dev] [11] RFR: [JDK-4954348]: JPGWriter.getNumThumbnailsSupported does not return -1 when passing null values On 3/23/18, 2:21 AM, Jayathirth D V wrote: > Hi Prahalad, > > Change looks fine. > Test case works properly before and after the code change. ? You mean it does not fail before the fix ? It definitely fails for me before the fix. > > Since we are touching getNumThumbnailsSupported() function and it is only > overridden in JPEGImageWriter we can add override annotation for the same. I would be more inclined to address that in a separate bug that adds the annotation for all the over-ridden JPEG reader+writer methods, rather than adding just one in the file. -phil. > Thanks, > Jay > > -Original Message- > From: Prahalad Kumar Narayanan > Sent: Friday, March 23, 2018 1:40 PM > To: 2d-dev > Subject: [OpenJDK 2D-Dev] [11] RFR: [JDK-4954348]: > JPGWriter.getNumThumbnailsSupported does not return -1 when passing > null values > > Hello Everyone > > Good day to you. > > Request your time to review a simple fix for the bug > Bug: JDK-4954348 (https://bugs.openjdk.java.net/browse/JDK-4954348) > Title: JPGWriter.getNumThumbnailsSupported does not return -1 > when passing null values > > Root Cause: > . As per ImageWriter specification, the method- > getNumThumbnailsSupported, should return -1 when invoked with insufficient > data. > . However, the method's implementation in JpegImageWriter returns 0 > instead of -1 in this use-case. > > Information on the fix: > . The logic within concerned method of Jpeg image writer requires one of > the two inputs- ImageTypeSpecifer (or) IIOMetadata. > . If both the required inputs are 'Null' the method has insufficient > data& thus cannot validate. > . A simple check is now added in the method to detect this use-case and > return -1 as recommended in the specification. > > Kindly review the changes at your convenience > Link: http://cr.openjdk.java.net/~pnarayanan/4954348/webrev.00 > > Thank you > Have a good day > > Prahalad N.
Re: [OpenJDK 2D-Dev] [11] RFR: [JDK-4954348]: JPGWriter.getNumThumbnailsSupported does not return -1 when passing null values
On 3/23/18, 11:56 AM, Sergey Bylokhov wrote: Hi, Prahalad. A few small comments about the test: - Is it possible to test all installed ImageWriterSpi? It seems that the test itself is not a JPEG plugin specific? None of the other plugins support thumbnails so they will all return 0 regardless. So I think that aspect of the test is OK - You will need to dispose the jpgWriter even in case of exception. Probably should fix that. Everything else seems fine. -phil. On 23/03/2018 01:10, Prahalad Kumar Narayanan wrote: Kindly review the changes at your convenience Link: http://cr.openjdk.java.net/~pnarayanan/4954348/webrev.00 Thank you Have a good day Prahalad N.
Re: [OpenJDK 2D-Dev] [11] RFR: [JDK-4954348]: JPGWriter.getNumThumbnailsSupported does not return -1 when passing null values
On 3/23/18, 2:21 AM, Jayathirth D V wrote: Hi Prahalad, Change looks fine. Test case works properly before and after the code change. ? You mean it does not fail before the fix ? It definitely fails for me before the fix. Since we are touching getNumThumbnailsSupported() function and it is only overridden in JPEGImageWriter we can add override annotation for the same. I would be more inclined to address that in a separate bug that adds the annotation for all the over-ridden JPEG reader+writer methods, rather than adding just one in the file. -phil. Thanks, Jay -Original Message- From: Prahalad Kumar Narayanan Sent: Friday, March 23, 2018 1:40 PM To: 2d-dev Subject: [OpenJDK 2D-Dev] [11] RFR: [JDK-4954348]: JPGWriter.getNumThumbnailsSupported does not return -1 when passing null values Hello Everyone Good day to you. Request your time to review a simple fix for the bug Bug: JDK-4954348 (https://bugs.openjdk.java.net/browse/JDK-4954348) Title: JPGWriter.getNumThumbnailsSupported does not return -1 when passing null values Root Cause: . As per ImageWriter specification, the method- getNumThumbnailsSupported, should return -1 when invoked with insufficient data. . However, the method's implementation in JpegImageWriter returns 0 instead of -1 in this use-case. Information on the fix: . The logic within concerned method of Jpeg image writer requires one of the two inputs- ImageTypeSpecifer (or) IIOMetadata. . If both the required inputs are 'Null' the method has insufficient data& thus cannot validate. . A simple check is now added in the method to detect this use-case and return -1 as recommended in the specification. Kindly review the changes at your convenience Link: http://cr.openjdk.java.net/~pnarayanan/4954348/webrev.00 Thank you Have a good day Prahalad N.
Re: [OpenJDK 2D-Dev] [11] RFR: [JDK-4954348]: JPGWriter.getNumThumbnailsSupported does not return -1 when passing null values
Hi, Prahalad. A few small comments about the test: - Is it possible to test all installed ImageWriterSpi? It seems that the test itself is not a JPEG plugin specific? - You will need to dispose the jpgWriter even in case of exception. On 23/03/2018 01:10, Prahalad Kumar Narayanan wrote: Kindly review the changes at your convenience Link: http://cr.openjdk.java.net/~pnarayanan/4954348/webrev.00 Thank you Have a good day Prahalad N. -- Best regards, Sergey.
Re: [OpenJDK 2D-Dev] [11] RFR: [JDK-4954348]: JPGWriter.getNumThumbnailsSupported does not return -1 when passing null values
Hi Prahalad, Change looks fine. Test case works properly before and after the code change. Since we are touching getNumThumbnailsSupported() function and it is only overridden in JPEGImageWriter we can add override annotation for the same. Thanks, Jay -Original Message- From: Prahalad Kumar Narayanan Sent: Friday, March 23, 2018 1:40 PM To: 2d-dev Subject: [OpenJDK 2D-Dev] [11] RFR: [JDK-4954348]: JPGWriter.getNumThumbnailsSupported does not return -1 when passing null values Hello Everyone Good day to you. Request your time to review a simple fix for the bug Bug: JDK-4954348 (https://bugs.openjdk.java.net/browse/JDK-4954348) Title: JPGWriter.getNumThumbnailsSupported does not return -1 when passing null values Root Cause: . As per ImageWriter specification, the method- getNumThumbnailsSupported, should return -1 when invoked with insufficient data. . However, the method's implementation in JpegImageWriter returns 0 instead of -1 in this use-case. Information on the fix: . The logic within concerned method of Jpeg image writer requires one of the two inputs- ImageTypeSpecifer (or) IIOMetadata. . If both the required inputs are 'Null' the method has insufficient data & thus cannot validate. . A simple check is now added in the method to detect this use-case and return -1 as recommended in the specification. Kindly review the changes at your convenience Link: http://cr.openjdk.java.net/~pnarayanan/4954348/webrev.00 Thank you Have a good day Prahalad N.
Re: [OpenJDK 2D-Dev] [11] RFR: [JDK-4954348]: JPGWriter.getNumThumbnailsSupported does not return -1 when passing null values
Hi Prahalad, The changes look fine to me. Thanks, Krishna -Original Message- From: Prahalad Kumar Narayanan Sent: Friday, March 23, 2018 1:40 PM To: 2d-dev <2d-dev@openjdk.java.net> Subject: [OpenJDK 2D-Dev] [11] RFR: [JDK-4954348]: JPGWriter.getNumThumbnailsSupported does not return -1 when passing null values Hello Everyone Good day to you. Request your time to review a simple fix for the bug Bug: JDK-4954348 (https://bugs.openjdk.java.net/browse/JDK-4954348) Title: JPGWriter.getNumThumbnailsSupported does not return -1 when passing null values Root Cause: . As per ImageWriter specification, the method- getNumThumbnailsSupported, should return -1 when invoked with insufficient data. . However, the method's implementation in JpegImageWriter returns 0 instead of -1 in this use-case. Information on the fix: . The logic within concerned method of Jpeg image writer requires one of the two inputs- ImageTypeSpecifer (or) IIOMetadata. . If both the required inputs are 'Null' the method has insufficient data & thus cannot validate. . A simple check is now added in the method to detect this use-case and return -1 as recommended in the specification. Kindly review the changes at your convenience Link: http://cr.openjdk.java.net/~pnarayanan/4954348/webrev.00 Thank you Have a good day Prahalad N.
