HI Andrew,
I have some minor comments on webrev.05. (webrev.06 looks rather strange.)
Thanks,
Brian
Note: Hotspot changes not reviewed here.
General
Check copyrights. Some headers are missing altogether, some have incorrect
years.
MappedByteBuffer.java
84: “any of other modes” -> “any of the other modes”, “this” -> “This”
85: nit: usually use American spelling: “behaviour” -> “behavior” (but it
does not matter so much here as it is not public javadoc.
172: “pasing” -> “passing”
175: delete “using”
181: comma or semicolon before “otherwise”
355-365: Collapse to?
Unsafe.getUnsafe().writebackMemory(address + index, length);
Unsafe.java
924-945: Capitalize first words of sentences.
959: Is there a possibility of overflow, e.g., use
Math.addExact(address, length) ?
1290: Insert blank line after.
FileChannelImpl.c (Unix)
85: “||! map_sync” -> “”|| !map_sync”
98-110: Put symbolic name definitions at head of file?
FileChannelImpl.c (Windows)
91: Use same message as at Unix version line 135?
ExtendedMapMode
13: Constant name should be MAP_MODE_CONSTRUCTOR.
13: Insert blank line after
34-36: Move to before line 13.
24: Move constructor to end of class.
> On Jun 3, 2019, at 7:37 AM, Andrew Dinn <ad...@redhat.com> wrote:
>
>
>
> On 03/06/2019 14:17, Andrew Dinn wrote:
>> Hi Vladimir,
>>
>> Thanks for the follow-up and for checking the submit failure. I have
>> addressed all the points you raised (point by point comments are
>> included below). A new webrev which passes a submit run is here:
>>
>> http://cr.openjdk.java.net/~adinn/8224974/webrev.04
>>
>> n.b. This webrev also includes changes to the javadoc for
>> ExtendedMapMode suggested by Alan Bateman.
> . . .
>
> I have made one further change to locate the new ExtendedMapMode enum in
> module jdk.nio.mapmode and package jdk.nio.mapmode as suggested offline
> by Alan Bateman and the OpenJDK lead. Please refer to this webrev instead:
>
> webrev: http://cr.openjdk.java.net/~adinn/8224974/webrev.05
>
> The CSR and JEP have been updated accordingly
>
> CSR: https://bugs.openjdk.java.net/browse/JDK-8224975
> JEP: https://bugs.openjdk.java.net/browse/JDK-8207851
>
> regards,
>
>
> Andrew Dinn
> -----------
> Senior Principal Software Engineer
> Red Hat UK Ltd
> Registered in England and Wales under Company Registration No. 03798903
> Directors: Michael Cunningham, Michael ("Mike") O'Neill, Eric Shander
