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