Looks good to me too; and I'd agree with Bengt that the code really belongs under "consistency checking" rather than under "parsing".<br><br>-- ramki<br><br><div class="gmail_quote">On Fri, Sep 14, 2012 at 5:31 PM, John Cuthbertson <span dir="ltr"><<a href="mailto:john.cuthbertson@oracle.com" target="_blank">john.cuthbertson@oracle.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi Bengt,<br>
<br>
Thanks for the review. I too went back and forth about where to put the check. I'm not sure there is a clear division but I'll see which location looks a bitter 'fit'.<br>
<br>
JohnC<div class="HOEnZb"><div class="h5"><br>
<br>
On 09/13/12 23:39, Bengt Rutisson wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Hi John,<br>
<br>
Thanks for fixing this!<br>
<br>
It looks good to me. I'm not really sure about how the work in arguments.cpp is supposed to be divided. You added the new check to Arguments::parse() but I guess it would be possible to put the code into Snippet Arguments::check_vm_args_<u></u>consistency() instead. It looks to me like it might fit better there, but I am fine with leaving it in Arguments::parse() as well. Just glad to get rid of the duplicated code.<br>

<br>
Thanks,<br>
Bengt<br>
<br>
<br>
On 2012-09-13 18:58, John Cuthbertson wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Hi Everyone,<br>
<br>
Can I have a couple of volunteers review the changes for this CR? They are fairly small. The webrev can be found at: <a href="http://cr.openjdk.java.net/%7Ejohnc/7193946/webrev.0/" target="_blank">http://cr.openjdk.java.net/~<u></u>johnc/7193946/webrev.0/</a><br>

<br>
Summary:<br>
In the review comments for the fix for 7192128, Bengt suggested to move the individual warnings from concurrentMarkSweepGeneration.<u></u>cpp and g1Collectedheap.cpp and place a single warning in a common piece of code. These changes address that review comment. The suggested location (vm_version_sparc.cpp) was unsuitable as the routine which would be the natural choice is called twice and the warning would be issued twice. A better place is when we check the other GC flags for consistency in arguments.cpp.<br>

<br>
Testing:<br>
* command line testing<br>
* GC basher and GCOld on sun4v with and without UseMemSetInBOT set<br>
<br>
Thanks,<br>
<br>
JohnC<br>
</blockquote>
<br>
</blockquote>
<br>
</div></div></blockquote></div><br>