[OpenJDK 2D-Dev] JDK-8013810: UnixPrintServiceLookup not returning consistent values

[Patrick Reinhart]

Hi Jennifer,
Hi Phil,

How big are the changes to get that to jdk7 update as well? The company 
I work for has a lot of customers that this fix would help.

Cheers Patrick

On 06/06/2013 07:35 PM, Jennifer Godinez wrote:
> Fix looks good.
>
> Jennifer
>
> On 06/01/2013 01:02 AM, Patrick Reinhart wrote:
>> Hi Phil,
>>
>> As I'm now back from my holidays I changed the fix according your 
>> suggestions:
>>
>> http://reinharts.dyndns.org/8013810/v3/webrev
>>
>> Can you take a look at it? I also restructured my webrev web folders 
>> for all older revisions.
>>
>> Cheers
>>
>> Patrick
>>
>> On 05/10/2013 12:08 AM, Phil Race wrote:
>>> Patrick,
>>> Maybe you need to be clear in the problem statement. I can't actually
>>> find it anywhere in this email thread.  I'm reverse engineering to 
>>> it as follows:
>>>
>>> You got an IPPPrintService for lookup via
>>>
>>> PrintServiceLookup.lookupPrintServices(null, null)
>>>
>>>
>>> but a UnixPrintService via
>>>
>>> attributes.add(new PrinterName(name, null));
>>> PrintServiceLookup.lookupPrintServices(null, attributes)
>>>
>>>
>>> If you read my email you should see that if you make the change
>>> I proposed you will never enter the getNamedPrinterXXX() methods
>>> if you are using CUPS, which seems like it should fix your problem
>>> of different classes.
>>>
>>> You don't really need to test the class and probably shouldn't.
>>> service.equals(serviceByName) seems better.
>>>
>>> And although getServiceByName() shouldn't enumerate all printers,
>>> because that can be very costly in some configurations,
>>> it likely should first check to see if we already enumerated all 
>>> printers,
>>> and then see if its in the list. As its written its really intended 
>>> for the
>>> case where no enumeration has been performed but if it already
>>> happened there's no harm in leveraging that.
>>>
>>> Put another, way does this patch (on its own, no other changes) fix 
>>> your problem
>>> It modifies the getPrinterByName() method. If not, why, not, and 
>>> please print
>>> out the actual class/service names seen by your test so we can see 
>>> more clearly
>>>
>>> diff --git 
>>> a/src/solaris/classes/sun/print/UnixPrintServiceLookup.java 
>>> b/src/solaris/classes/sun/print/UnixPrintServiceLookup.java
>>> --- a/src/solaris/classes/sun/print/UnixPrintServiceLookup.java
>>> +++ b/src/solaris/classes/sun/print/UnixPrintServiceLookup.java
>>> @@ -366,6 +366,29 @@
>>>          if (name == null || name.equals("") || 
>>> !checkPrinterName(name)) {
>>>              return null;
>>>          }
>>> +
>>> +        if (printServices != null) {
>>> +            for (int i=0; i<printServices.length; i++) {
>>> +                if (name.equals(printServices[i].getName())) {
>>> +                    return printServices[i];
>>> +                }
>>> +            }
>>> +        }
>>> +
>>> +        if (CUPSPrinter.isCupsRunning()) {
>>> +            try {
>>> +                URL url = new URL("http://"+
>>> + CUPSPrinter.getServer()+":"+
>>> +                                  CUPSPrinter.getPort()+"/"+
>>> +                                  name);
>>> +                printer = new IPPPrintService(name, url);
>>> +            } catch (Exception e) {
>>> +            }
>>> +            if (printer != null) {
>>> +                return printer;
>>> +            }
>>> +        }
>>> +
>>>          if (isMac() || isSysV()) {
>>>              printer = getNamedPrinterNameSysV(name);
>>>          } else {
>>>
>>>
>>> BTW keep all revisions of your code around so that people can compare.
>>> I have no reference to what the earlier version of your fix did.
>>>
>>> -phil.
>>
>