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

[Sergey Bylokhov]

+1

On 8/16/19 12:34 pm, Phil Race wrote:
> Approved.
> 
> -phil.
> 
> On 8/16/19 5:23 AM, Alexey Ivanov wrote:
>> 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.
>>
> 


-- 
Best regards, Sergey.