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