Hi,
On 8/23/2012 10:23 PM, Sean Chou wrote:
Hi Phil,

I'm really sorry about this typo, the modification looks so simple that I became careless when porting.

That's a slippery slope. Only send out code that you built and tested.
What if reviewer also thinks "this must be OK else it wouldn't have built, and of course he built it ... "

The patch is from ibmjdk and it has been tested on Java6 since Oct, 2007 and on Java7 since Feb, 2012. To be honest, this modification isn't related to a real bug in openjdk for now; it is posted to see if there are any reason that the printer is assumed to be a postscript printer in all exceptions.

The code went into JDK 6u2 on 27th Feb 2007 and would have been released a few months later.
IBM apparently made this change pretty soon after that.
So the question that should be asked is not why is the openjdk code like this,
but why did IBM make the change they did ?
I have no record or recollection of any problem reports.
I can't see any value to it. Unless there's a reflection problem (which there should not be!)
its never going to get executed.

-phil.


    Many thanks.

The false is corrected in this webrev:
http://cr.openjdk.java.net/~zhouyx/OJDK-429/webrev.02/ <http://cr.openjdk.java.net/%7Ezhouyx/OJDK-429/webrev.02/>

On Fri, Aug 24, 2012 at 12:20 AM, Phil Race <[email protected] <mailto:[email protected]>> wrote:

    Sean,

    Without even commenting on the merits or necessity I note that you
    cannot possibly
    have even built this patch, much less tested it.

    > 625 return Boolean.FLASE;

    -phil.


    On 8/23/12 1:58 AM, Sean Chou wrote:
    Hello,

        I updated the repository to 2d, the webrev is now:
    http://cr.openjdk.java.net/~zhouyx/OJDK-429/webrev.01/
    <http://cr.openjdk.java.net/%7Ezhouyx/OJDK-429/webrev.01/>
    Please take a look.

    ---------- Forwarded message ----------
    From: *Sean Chou* <[email protected]
    <mailto:[email protected]>>
    Date: Thu, Aug 23, 2012 at 2:24 PM
    Subject: Suggest a modification to isPostscript exception handling
    To: [email protected] <mailto:[email protected]>


    Hello,

         This is a simple modification to sun/print/PSPrinterJob.java.
         When sun.print.IPPPrintService.isPostscript method checks if
    the printer is a postscript printer, if IOException happens, the
    method assumes the printer is postscript printer
    (IPPPrintService.java, line 1605). In class PSPrinterJob, it
    invoke isPostscript and assumes all  Throwables to be a
    postscript printer ( PSPrinterJob.java, line 625 ).  I think it
    should return false in cases exceptions other
    than IOException are caught, IOException should not be expanded
    to all Throwable.

    The webrev is at:
    http://cr.openjdk.java.net/~zhouyx/OJDK-429/webrev.00/
    <http://cr.openjdk.java.net/%7Ezhouyx/OJDK-429/webrev.00/>  .

    Please take a look.

-- Best Regards,
    Sean Chou





--
Best Regards,
Sean Chou


Reply via email to