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

[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 :

http://cr.openjdk.java.net/~jdv/7059970/webrev.03/ <http://cr.openjdk.java.net/%7Ejdv/7059970/webrev.03/>

Thanks,

Jay

*From:*Prasanta Sadhukhan
*Sent:* Wednesday, July 13, 2016 12:35 PM
*To:* Jayathirth D V
*Cc:* 2d-dev

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

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,

    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/
    <http://cr.openjdk.java.net/%7Ejdv/7059970/webrev.02/>

    Thanks,

    Jay

    *From:*Brian Burkhalter
    *Sent:* Friday, July 08, 2016 11:35 PM
    *To:* Jayathirth D V
    *Cc:* Philip Race; 2d-dev
    *Subject:* Re: [OpenJDK 2D-Dev] Review Request for JDK-7059970 :
    Test case: javax/imageio/plugins/png/ITXtTest.java is not closing
    a file

    Hi Jay,

    Sorry to be picky here but in doTest() could you not instead have

        try {

            writeTo(file, src);

            ITXtTest dst = readFrom(file);

            if (dst == null || !dst.equals(src)) {

                throw new RuntimeException("Test failed.");

            }

        } catch (RuntimeException re) {

            throw re;

        } finally {

            file.delete();

        }

        System.out.println("Test passed.");

    and therefore remove f.delete() from writeTo() and readFrom()?

    Thanks,

    Brian

    On Jul 8, 2016, at 12:04 AM, Jayathirth D V
    <jayathirth....@oracle.com <mailto:jayathirth....@oracle.com>> wrote:




        I can’t perform f.delete() in finally block of  writeTo() and
        readFrom() because “test.png” is shared resource between the
        methods. So I am deleting “test.png” at places where we are
        throwing RumtimeException.