[OpenJDK 2D-Dev] [13] RFR 8222108: Reduce minRefreshTime for updating remote printer list on Windows

[Alexey Ivanov]

Hi Phil, Sergey,

Do you have any other comments?
The latest webrev:
http://cr.openjdk.java.net/~aivanov/8222108/webrev.02/

Does anybody have any additional comments?


I looked through the code once again, and I think there should be no 
null values in the array returned by getPrinterNames() unless 
::EnumPrinters could return null printer names. The documentation for 
EnumPrinters [1] and for PRINTER_INFO_4 [2] does not mention if 
pPrinterName can be null or not.

Anyway the custom comparator does handle null values in the printer list 
if there are any.

Regards,
Alexey

[1] https://docs.microsoft.com/en-us/windows/win32/printdocs/enumprinters
[2] https://docs.microsoft.com/en-us/windows/win32/printdocs/printer-info-4

On 03/07/2019 19:46, Alexey Ivanov wrote:
> Hi Phil,
>
> Thank you for your review! That's a valid point!
>
> Please see the updated webrev:
> http://cr.openjdk.java.net/~aivanov/8222108/webrev.02/
>
> I implemented a custom comparator which handles null values in the 
> printer list.
>
> However, I wonder if the list of printers can ever contain a null 
> value. The method refreshServices() does not check if printers[p] is 
> null.
>
> On 03/07/2019 00:13, Philip Race wrote:
>> I thought we had the checks for null in doCompare there for a reason.
>> Arrays.sort won't be very happy with a null element.
>>
>> You said in the first review email of the v0 webrev :
>>
>> > Arrays.equals() accepts null parameters and null elements in the 
>> arrays and always returns the correct result.
>>
>> but that webrev didn't use Arrays.sort and that requirement seems to 
>> have been forgotten
>> when adding it.
>>
>>  public class Sort {
>>   public static void main(String[] args) {
>>     String[] a1 = { "a", null, "a" };
>>     java.util.Arrays.sort(a1);
>>   }
>> }
>>
>> java Sort
>> Exception in thread "main" java.lang.NullPointerException
>>     at 
>> java.util.ComparableTimSort.countRunAndMakeAscending(ComparableTimSort.java:320)
>>     at java.util.ComparableTimSort.sort(ComparableTimSort.java:188)
>>     at java.util.Arrays.sort(Arrays.java:1246)
>>     at Sort.main(Sort.java:4)
>>
>>
>> -phil.