[OpenJDK 2D-Dev] Request for review: [NEW BUG] Printer spoolers ignore result from spool process

[Phil Race]

Maybe its just the style you are using that looks weird to me.
I prefer that you indent the subsequent parameter lines to match
the first parameter even if it adds white space.

I also prefer that foo and bar line up when writing expressions like
the following where there are > 1 lines in the (..) expression and
that you move the block to a new  line like this ..
although I can't promise email will line this up right.

try ( foo;
        bar)
{
// body
}

-phil.

On 10/20/11 12:38 PM, Phil Race wrote:
> PS you may want to look at your indentation. It appears inconsistent.
> Always use exactly 4 spaces for indentation.
>
> -phil.
>
> On 10/20/2011 12:20 PM, Phil Race wrote:
>> Neil,
>>
>> You are throwing an IOException and we already have the code to catch
>> that and re-throw as the documented printer exception type, and also 
>> the event should
>> get notified to any listeners.  So far so good.
>>
>> But you are throwing the IOException before calling 
>> spoolFile.delete() so we'll leak
>> the spool file.
>> Arguably that could happen anyway if there were an Exception but that 
>> was
>> probably not occurring previously.
>>
>> I suggest either to move the spoolfile.delete() to *before* the 
>> handleError call
>> or into a finally { } block.
>>
>> -phil.
>>
>>
>> On 10/20/2011 11:56 AM, Neil Richards wrote:
>>> Hi all,
>>> Whilst trying to debug a printing problem, I noticed that the (Unix and
>>> PostScript) printer spoolers in Java do not check what the result is of
>>> trying to launch the OS print spooler command (often 'lpr' or 'lp').
>>>
>>> As a result, if that exec'd command fails for any reason, that result
>>> (and that reason) is lost, and the user left without any clue that
>>> something is amiss.
>>>
>>> To address this, I've created a suggested fix [1], which checks the 
>>> exit
>>> code for the exec'd command and, if it's bad, throws an exception which
>>> captures any text from the command's error stream.
>>>
>>> It does this in both sun.print.PSPrinterJob and 
>>> sun.print.UnixPrinterJob
>>> (they are very similar in composition and function).
>>>
>>> Please review this suggested change.
>>>
>>> Thanks, Neil
>>>
>>> [1] http://cr.openjdk.java.net/~ngmr/ojdk-201/webrev.01/index.html
>>>
>>
>