Re: [OpenJDK 2D-Dev] Review Request for JDK-7059970 : Test case: javax/imageio/plugins/png/ITXtTest.java is not closing a file

2016-07-13 Thread Jayathirth D V
Hi Prasanta, Please find updated webrev for reference : http://cr.openjdk.java.net/~jdv/7059970/webrev.04/ Thanks, Jay From: Prasanta Sadhukhan Sent: Wednesday, July 13, 2016 5:16 PM To: Jayathirth D V Cc: 2d-dev Subject: Re: [OpenJDK 2D-Dev] Review Request for JDK-7059970 : Test

Re: [OpenJDK 2D-Dev] Review Request for JDK-7059970 : Test case: javax/imageio/plugins/png/ITXtTest.java is not closing a file

2016-07-13 Thread Prasanta Sadhukhan
Looks good. Onlything is, in copyright you need to add a comma after 2016. Regards Prasanta On 7/13/2016 5:03 PM, Jayathirth D V wrote: Hi Prasanta, There is no need to capture and throw the same excpetion.I have applied changes mentioned by you. Please find updated webrev for review :

Re: [OpenJDK 2D-Dev] Review Request for JDK-7059970 : Test case: javax/imageio/plugins/png/ITXtTest.java is not closing a file

2016-07-13 Thread Jayathirth D V
Hi Prasanta, There is no need to capture and throw the same excpetion.I have applied changes mentioned by you. Please find updated webrev for review : http://cr.openjdk.java.net/~jdv/7059970/webrev.03/ Thanks, Jay From: Prasanta Sadhukhan Sent: Wednesday, July 13, 2016 12:35 PM

Re: [OpenJDK 2D-Dev] Review Request for JDK-7059970 : Test case: javax/imageio/plugins/png/ITXtTest.java is not closing a file

2016-07-13 Thread Prasanta Sadhukhan
Hi Jay, Why do we need to catch and throw RuntimeExcpn? try { } catch (RuntimeException e) { throw e; } Can't we just do try { } finally { } and let the exception be thrown naturally without being caught and rethrown? Regards Prasanta On 7/12/2016 5:33 PM, Jayathirth D V wrote: Hi Brian,

Re: [OpenJDK 2D-Dev] Review Request for JDK-7059970 : Test case: javax/imageio/plugins/png/ITXtTest.java is not closing a file

2016-07-12 Thread Brian Burkhalter
Hi Jay, Looks fine to me. Thanks, Brian On Jul 12, 2016, at 5:03 AM, Jayathirth D V wrote: > That’s very good thing to do as it will remove redundant f.delete(). > I have updated the webrev for review : > http://cr.openjdk.java.net/~jdv/7059970/webrev.02/

Re: [OpenJDK 2D-Dev] Review Request for JDK-7059970 : Test case: javax/imageio/plugins/png/ITXtTest.java is not closing a file

2016-07-07 Thread Brian Burkhalter
Good point - probably better to make it explicit then. The try-with-resources should still be cleaner for closing the streams however. Brian On Jul 7, 2016, at 1:26 PM, Phil Race wrote: > I suppose deleteOnExit might be OK but jtreg re-uses the VM > so it will not get

Re: [OpenJDK 2D-Dev] Review Request for JDK-7059970 : Test case: javax/imageio/plugins/png/ITXtTest.java is not closing a file

2016-07-07 Thread Phil Race
I suppose deleteOnExit might be OK but jtreg re-uses the VM so it will not get deleted until much later - after all N thousand tests have been run .. assuming no other test crashes the VM before then. -phil. On 07/07/2016 10:41 AM, Brian Burkhalter wrote: Why not simply invoke deleteOnExit()

Re: [OpenJDK 2D-Dev] Review Request for JDK-7059970 : Test case: javax/imageio/plugins/png/ITXtTest.java is not closing a file

2016-07-07 Thread Phil Race
you need to add the new bug to @bug .. 197 imageWriter.write(new IIOImage(src, null, m)); 198 imageOutputStream.close(); <<< 199 System.out.println("Writing done."); 200 } catch (Throwable e) { 201 if (imageOutputStream

Re: [OpenJDK 2D-Dev] Review Request for JDK-7059970 : Test case: javax/imageio/plugins/png/ITXtTest.java is not closing a file

2016-07-07 Thread Brian Burkhalter
Why not simply invoke deleteOnExit() on the file instance immediately after it is created and use try-with-resources blocks for the Image{Input,Output}Stream instances? Brian On Jul 7, 2016, at 4:38 AM, Jayathirth D V wrote: > Please review the following fix in

[OpenJDK 2D-Dev] Review Request for JDK-7059970 : Test case: javax/imageio/plugins/png/ITXtTest.java is not closing a file

2016-07-07 Thread Jayathirth D V
Hi, Please review the following fix in JDK9 : Bug : https://bugs.openjdk.java.net/browse/JDK-7059970 Webrev : http://cr.openjdk.java.net/~jdv/7059970/webrev.00/ Root cause : Test case ITXtTest.java is not deleting the file it is creating(test.png). Also it is not closing