[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-2d-dev
Subject:    Re: [OpenJDK 2D-Dev] [13] RFR 8222108: Reduce minRefreshTime for updating remote printer list on Win
From:       Alexey Ivanov <alexey.ivanov () oracle ! com>
Date:       2019-07-03 16:46:40
Message-ID: 178f2ca9-ea7d-b170-da36-e73966400a6a () oracle ! com
[Download RAW message or body]

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.
>
> On 7/2/19, 5:18 AM, Alexey Ivanov wrote:
>> Are there any other comments?
>> Other volunteers for reviewing?
>>
>> Thanks in advance.
>>
>> On 25/06/2019 20:17, Alexey Ivanov wrote:
>>> Please see the updated webrev:
>>> http://cr.openjdk.java.net/~aivanov/8222108/webrev.01/
>>>
>>> <SNIP>
-- 
Regards,
Alexey
[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic