<div dir="ltr"><div dir="ltr">Hi Chris,<div><br></div><div>Sounds good to me; here it is:</div><div>Webrev:Â <a href="http://cr.openjdk.java.net/~jcbeyler/8210689/webrev.01/">http://cr.openjdk.java.net/~jcbeyler/8210689/webrev.01/</a></div><div>Bug:Â <a href="https://bugs.openjdk.java.net/browse/JDK-8210689" target="_blank">https://bugs.openjdk.java.net/browse/JDK-8210689</a></div><div><br></div><div>I admit I strived to stay consistent and always started a new line for the multi-line argument even if the string was not too long; it's a question of style I believe but it felt more readable to me. I'll happily change whatever anyone prefers.</div><div><br></div><div>This has passed the vmTestbase tests I changed but due to the shared changes, I've launched a full vmTestbase testing now.</div><div><br></div><div>Let me know what you think,</div><div>Jc</div></div></div><br><div class="gmail_quote"><div dir="ltr">On Fri, Sep 21, 2018 at 10:59 AM Chris Plummer <<a href="mailto:chris.plummer@oracle.com">chris.plummer@oracle.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div text="#000000" bgcolor="#FFFFFF">
<div class="m_4728043626513501693moz-cite-prefix">On 9/21/18 10:55 AM, JC Beyler wrote:<br>
</div>
<blockquote type="cite">
<div dir="ltr">Hi Chris,
<div><br>
</div>
<div>I hesitated to be honest and then thought that debug_str
was better as you would clearly see that it is a multi-lilne
string and what parameters are what. But I'll take your
preference (it's relatively the same for me). Quick question
though:</div>
<div><br>
</div>
<div>Do you have a preference between:</div>
<div>
<pre id="m_4728043626513501693gmail-hterm:copy-to-clipboard-source" style="color:rgb(0,0,0)"> NSK_COMPLAIN6(
"TEST FAILED: %s field \"%s\" has\n"
"\tsignature: \"%s\"\n"
"\tgeneric signature: \"%s\"\n\n"
"\tExpected: \"%s\"\n"
"\t\t\"%s\"\n\n",
(instance==0)?"instance":"static",
fld_sig[idx][0],
sign, (gen_sign==NULL)?"NULL":gen_sign,
fld_sig[idx][2], fld_sig[idx][3]);</pre>
<pre id="m_4728043626513501693gmail-hterm:copy-to-clipboard-source" style="color:rgb(0,0,0)">
</pre>
<pre id="m_4728043626513501693gmail-hterm:copy-to-clipboard-source" style="color:rgb(0,0,0)">or:</pre>
<pre id="m_4728043626513501693gmail-hterm:copy-to-clipboard-source" style="color:rgb(0,0,0)"><pre id="m_4728043626513501693gmail-hterm:copy-to-clipboard-source"> NSK_COMPLAIN6(
"TEST FAILED: %s field \"%s\" has\n\tsignature: \"%s\"\n"
"\tgeneric signature: \"%s\"\n\n\tExpected: \"%s\"\n\t\t\"%s\"\n\n",
(instance==0)?"instance":"static",
fld_sig[idx][0],
sign, (gen_sign==NULL)?"NULL":gen_sign,
fld_sig[idx][2], fld_sig[idx][3]);</pre><pre id="m_4728043626513501693gmail-hterm:copy-to-clipboard-source">
</pre><pre id="m_4728043626513501693gmail-hterm:copy-to-clipboard-source">I think I like the first because you can clearly see what we want to be printed out; but for code vertical</pre><pre id="m_4728043626513501693gmail-hterm:copy-to-clipboard-source">compression, the second is better. What do you think?</pre></pre>
</div>
</div>
</blockquote>
I also prefer the first one.<br>
<br>
thanks,<br>
<br>
Chris<br>
<blockquote type="cite">
<div dir="ltr">
<div>
<pre id="m_4728043626513501693gmail-hterm:copy-to-clipboard-source" style="color:rgb(0,0,0)"><pre id="m_4728043626513501693gmail-hterm:copy-to-clipboard-source">
</pre><pre id="m_4728043626513501693gmail-hterm:copy-to-clipboard-source">Thanks!</pre><pre id="m_4728043626513501693gmail-hterm:copy-to-clipboard-source">Jc</pre></pre>
</div>
</div>
<br>
<div class="gmail_quote">
<div dir="ltr">On Fri, Sep 21, 2018 at 10:16 AM Chris Plummer
<<a href="mailto:chris.plummer@oracle.com" target="_blank">chris.plummer@oracle.com</a>>
wrote:<br>
</div>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div text="#000000" bgcolor="#FFFFFF">
<div class="m_4728043626513501693m_-8718529575060114443moz-cite-prefix"><tt>Hi
JC,</tt><tt><br>
</tt><tt><br>
</tt><tt>I didn't realize you intended to move all the
strings into a "const char*" first. Seems unnecessary,
and I think not as easy to read:</tt><tt><br>
</tt><tt><br>
</tt><tt>Â 138Â Â Â Â Â Â Â Â const char* debug_str =</tt><tt><br>
</tt><tt>Â 139Â Â Â Â Â Â Â Â Â Â Â Â "TEST FAILED:
JVMTI_EVENT_CLASS_LOAD event received for\n"</tt><tt><br>
</tt><tt>Â 140Â Â Â Â Â Â Â Â Â Â Â Â "\t a primitive class/array of
primitive types with the signature \"%s\"\n";</tt><tt><br>
</tt><tt>Â 141Â Â Â Â Â Â Â Â NSK_COMPLAIN1(debug_str, sig);</tt><tt><br>
</tt><tt><br>
</tt><tt>vs</tt><tt><br>
</tt><tt><br>
</tt><tt>Â 138Â Â Â Â Â Â Â Â NSK_COMPLAIN1("TEST FAILED:
JVMTI_EVENT_CLASS_LOAD event received for\n"</tt><tt><br>
</tt><tt>Â 139Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â "\t a primitive
class/array of primitive types with the signature
\"%s\"\n",</tt><tt><br>
</tt><tt>Â 140Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â sig);</tt><tt><br>
<br>
or<br>
</tt><tt><tt><br>
</tt><tt>Â 138Â Â Â Â Â Â Â Â NSK_COMPLAIN1(<br>
 139            "TEST FAILED: JVMTI_EVENT_CLASS_LOAD
event received for\n"</tt><tt><br>
 </tt><tt>140            "\t a primitive class/array
of primitive types with the signature \"%s\"\n",</tt><tt><br>
 </tt><tt>141            sig);</tt><tt><br>
</tt></tt><tt><br>
</tt><tt>thanks,</tt><tt><br>
</tt><tt><br>
</tt><tt>Chris</tt><tt><br>
</tt><tt><br>
</tt><tt>On 9/21/18 8:00 AM, JC Beyler wrote:</tt><tt><br>
</tt></div>
<blockquote type="cite">
<div dir="ltr"><tt>Hi all,</tt><tt><br>
</tt>
<div><tt><br>
</tt></div>
<div><tt>Is anyone motivated on a Friday to review this
? :)</tt></div>
<div><tt><br>
</tt></div>
<div><tt>It should be fairly straightforward :-)</tt></div>
<div><tt><br>
</tt></div>
<div><tt>Thanks,</tt></div>
<div><tt>Jc</tt></div>
</div>
<tt><br>
</tt>
<div class="gmail_quote">
<div dir="ltr"><tt>On Tue, Sep 18, 2018 at 12:07 PM JC
Beyler <</tt><tt><a href="mailto:jcbeyler@google.com" target="_blank">jcbeyler@google.com</a></tt><tt>>
wrote:</tt><tt><br>
</tt></div>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr"><tt>Hi all,</tt>
<div><tt><br>
</tt></div>
<div><tt>Could I have a review for this
webrev:Â </tt></div>
<div><tt><br>
</tt></div>
<div><tt>Webrev:Â </tt><tt><a href="http://cr.openjdk.java.net/%7Ejcbeyler/8210689/webrev.00/" target="_blank">http://cr.openjdk.java.net/~jcbeyler/8210689/webrev.00/</a></tt></div>
<div><tt>Bug:Â </tt><tt><a href="https://bugs.openjdk.java.net/browse/JDK-8210689" target="_blank">https://bugs.openjdk.java.net/browse/JDK-8210689</a></tt></div>
<div><tt><br>
</tt></div>
<div><tt>Let me know what you think,</tt><tt><br>
</tt></div>
<div>
<div dir="ltr" class="m_4728043626513501693m_-8718529575060114443m_-8264595719550593181gmail_signature">
<div dir="ltr">
<div><tt>Jc</tt><tt><br>
</tt></div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</blockquote>
</div>
<tt><br clear="all">
</tt>
<div><tt><br>
</tt></div>
<tt>-- </tt><tt><br>
</tt>
<div dir="ltr" class="m_4728043626513501693m_-8718529575060114443gmail_signature" data-smartmail="gmail_signature">
<div dir="ltr">
<div><tt><br>
</tt></div>
<tt>Thanks,</tt>
<div><tt>Jc</tt></div>
</div>
</div>
</blockquote>
<p><tt><br>
</tt></p>
</div>
</blockquote>
</div>
<br clear="all">
<div><br>
</div>
-- <br>
<div dir="ltr" class="m_4728043626513501693gmail_signature" data-smartmail="gmail_signature">
<div dir="ltr">
<div><br>
</div>
Thanks,
<div>Jc</div>
</div>
</div>
</blockquote>
<p><br>
</p>
</div>
</blockquote></div><br clear="all"><div><br></div>-- <br><div dir="ltr" class="gmail_signature" data-smartmail="gmail_signature"><div dir="ltr"><div><br></div>Thanks,<div>Jc</div></div></div>