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

[Jennifer Godinez]

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.
>