<html>
<head>
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
Latest webrev is here: <a moz-do-not-send="true"
class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Ejprovino/8085983/webrev.00">http://cr.openjdk.java.net/~jprovino/8085983/webrev.01</a><br>
<br>
joe<br>
<br>
<div class="moz-cite-prefix">On 6/18/2015 10:52 AM, Joseph Provino
wrote:<br>
</div>
<blockquote cite="mid:5582DB1A.6070609@oracle.com" type="cite">
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
<br>
<div class="moz-cite-prefix">On 6/10/2015 12:46 PM, Stefan
Karlsson wrote:<br>
</div>
<blockquote cite="mid:557869E2.4080905@oracle.com" type="cite">
<meta content="text/html; charset=utf-8"
http-equiv="Content-Type">
Hi Joe,<br>
<br>
<div class="moz-cite-prefix">On 2015-06-08 21:16, Joseph Provino
wrote:<br>
</div>
<blockquote cite="mid:5575EA21.4010803@oracle.com" type="cite">One
file. <br>
<br>
CR: <a moz-do-not-send="true" class="moz-txt-link-freetext"
href="https://bugs.openjdk.java.net/browse/JDK-8085983">https://bugs.openjdk.java.net/browse/JDK-8085983</a>
<br>
<br>
webrev: <a moz-do-not-send="true"
class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Ejprovino/8085983/webrev.00">http://cr.openjdk.java.net/~jprovino/8085983/webrev.00</a>
<br>
</blockquote>
<br>
Some comments:<br>
<br>
<pre><b><font color="blue">+</font><font color="blue"> guarantee(!cl->doHeapRegion(cur), err_msg("doHeapRegion() returned true"));</font></b></pre>
<br>
- You don't need err_msg if you don't have any format specifiers
in the error message.<br>
<br>
</blockquote>
I'll fix that.<br>
<br>
<blockquote cite="mid:557869E2.4080905@oracle.com" type="cite"> -
I would prefer if you didn't put the cl->doHeapReagion()
calls inside guarantees. It's too easy to incorrectly assume
that the code in the guarantee is only verification code, IMHO.<br>
<br>
</blockquote>
That's a good point. Something like this:<br>
<br>
bool ret = cl->doHeapRegion(r);<br>
guarantee(ret == false, "doHeapRegion() returned true");<br>
<br>
Or do you prefer !ret?<br>
<br>
<blockquote cite="mid:557869E2.4080905@oracle.com" type="cite"> -
Shouldn't this be an assert instead of a guarantee?<br>
</blockquote>
I don't know the rules there. Seems like assert would be better<br>
so it doesn't slow down the product build.<br>
<br>
thanks.<br>
<br>
joe<br>
<br>
<blockquote cite="mid:557869E2.4080905@oracle.com" type="cite"> <br>
Thanks,<br>
StefanK<br>
<br>
<blockquote cite="mid:5575EA21.4010803@oracle.com" type="cite">
<br>
passes jprt. <br>
<br>
thanks. <br>
<br>
joe <br>
<br>
<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</body>
</html>