Hi Alex,
Good catch, it was not done on purpose but now fixed:
Webrev: http://cr.openjdk.java.net/~jcbeyler/8210689/webrev.03/
<http://cr.openjdk.java.net/%7Ejcbeyler/8210689/webrev.03/>
Bug: https://bugs.openjdk.java.net/browse/JDK-8210689
Let me know if this works for you and thanks for the review,
Jc
On Fri, Sep 21, 2018 at 3:44 PM Alex Menkov <alexey.men...@oracle.com
<mailto:alexey.men...@oracle.com>> wrote:
Hi Jc,
overall looks good (no changes in the logging)
except
test/hotspot/jtreg/vmTestbase/nsk/jvmti/scenarios/hotswap/HS201/hs201t003/hs201t003.cpp
:
- if ((strcmp(name, expMeth) == 0) &&
- (strcmp(sig, expSig) == 0)) {
- NSK_DISPLAY4("===== %s event received for the tested method:\n\
-\tID=0x%p name=\"%s\" signature=\"%s\"\n",
+ if ((strcmp(name, expMeth) == 0) && (strcmp(sig, expSig) == 0)) {
+ NSK_DISPLAY4(
+ "%s event received for the tested method:\n"
+ "\tID=0x%p name=\"%s\" signature=\"%s\"\n",
"===== " is dropped from the beginning of the line
I don't know if this is important.
--alex
On 09/21/2018 14:29, JC Beyler wrote:
> Hi Chris,
>
> Done here:
> Webrev: http://cr.openjdk.java.net/~jcbeyler/8210689/webrev.02/
<http://cr.openjdk.java.net/%7Ejcbeyler/8210689/webrev.02/>
> <http://cr.openjdk.java.net/%7Ejcbeyler/8210689/webrev.02/>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8210689
>
> Anything else? and anybody else motivated to look?
>
> Thanks again!
> Jc
>
> On Fri, Sep 21, 2018 at 2:07 PM Chris Plummer
<chris.plum...@oracle.com <mailto:chris.plum...@oracle.com>
> <mailto:chris.plum...@oracle.com
<mailto:chris.plum...@oracle.com>>> wrote:
>
> Hi JC,
>
> Overall looks good. Just a couple minor edits needed:
>
> In nativemethbind003.cpp:
>
> 158 NSK_DISPLAY1("Inside the registerNative()\nFinding
class
> \"%s\" ...\n", CLASS_SIG);
>
> This was two lines and you made it one with a \n in the middle of
> the string.
>
> In ap12t001.cpp:
>
> 69 NSK_COMPLAIN2(
> 70 "Received unexpected number of ObjectFree
events:
> %d\n"
> 71 "\texpected number: %d",
> 72 obj_free, EXP_OBJ_FREE);
>
> There's no \n at the end of this output (and there never was).
> Normally NSK_COMPLAIN is always used with a terminating \n.
>
> thanks,
>
> Chris
>
>
> On 9/21/18 1:05 PM, JC Beyler wrote:
>> Hi Chris,
>>
>> Sounds good to me; here it is:
>> Webrev:
http://cr.openjdk.java.net/~jcbeyler/8210689/webrev.01/
<http://cr.openjdk.java.net/%7Ejcbeyler/8210689/webrev.01/>
>> <http://cr.openjdk.java.net/%7Ejcbeyler/8210689/webrev.01/>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8210689
>>
>> 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.
>>
>> This has passed the vmTestbase tests I changed but due to the
>> shared changes, I've launched a full vmTestbase testing now.
>>
>> Let me know what you think,
>> Jc
>>
>> On Fri, Sep 21, 2018 at 10:59 AM Chris Plummer
>> <chris.plum...@oracle.com <mailto:chris.plum...@oracle.com>
<mailto:chris.plum...@oracle.com <mailto:chris.plum...@oracle.com>>>
wrote:
>>
>> On 9/21/18 10:55 AM, JC Beyler wrote:
>>> Hi Chris,
>>>
>>> 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:
>>>
>>> Do you have a preference between:
>>> 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]);
>>> or:
>>> 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]);
>>> I think I like the first because you can clearly see
what we want to be printed out; but for code vertical
>>> compression, the second is better. What do you think?
>> I also prefer the first one.
>>
>> thanks,
>>
>> Chris
>>> Thanks!
>>> Jc
>>>
>>> On Fri, Sep 21, 2018 at 10:16 AM Chris Plummer
>>> <chris.plum...@oracle.com
<mailto:chris.plum...@oracle.com> <mailto:chris.plum...@oracle.com
<mailto:chris.plum...@oracle.com>>>
>>> wrote:
>>>
>>> Hi JC,
>>>
>>> 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:
>>>
>>> 138 const char* debug_str =
>>> 139 "TEST FAILED: JVMTI_EVENT_CLASS_LOAD
>>> event received for\n"
>>> 140 "\t a primitive class/array of
primitive
>>> types with the signature \"%s\"\n";
>>> 141 NSK_COMPLAIN1(debug_str, sig);
>>>
>>> vs
>>>
>>> 138 NSK_COMPLAIN1("TEST FAILED:
>>> JVMTI_EVENT_CLASS_LOAD event received for\n"
>>> 139 "\t a primitive
class/array of
>>> primitive types with the signature \"%s\"\n",
>>> 140 sig);
>>>
>>> or
>>>
>>> 138 NSK_COMPLAIN1(
>>> 139 "TEST FAILED: JVMTI_EVENT_CLASS_LOAD
>>> event received for\n"
>>> 140 "\t a primitive class/array of
primitive
>>> types with the signature \"%s\"\n",
>>> 141 sig);
>>>
>>> thanks,
>>>
>>> Chris
>>>
>>> On 9/21/18 8:00 AM, JC Beyler wrote:
>>>> Hi all,
>>>>
>>>> Is anyone motivated on a Friday to review this ? :)
>>>>
>>>> It should be fairly straightforward :-)
>>>>
>>>> Thanks,
>>>> Jc
>>>>
>>>> On Tue, Sep 18, 2018 at 12:07 PM JC Beyler
>>>> <jcbey...@google.com <mailto:jcbey...@google.com>
<mailto:jcbey...@google.com <mailto:jcbey...@google.com>>> wrote:
>>>>
>>>> Hi all,
>>>>
>>>> Could I have a review for this webrev:
>>>>
>>>> Webrev:
>>>> http://cr.openjdk.java.net/~jcbeyler/8210689/webrev.00/
<http://cr.openjdk.java.net/%7Ejcbeyler/8210689/webrev.00/>
>>>>
<http://cr.openjdk.java.net/%7Ejcbeyler/8210689/webrev.00/>
>>>> Bug:
https://bugs.openjdk.java.net/browse/JDK-8210689
>>>>
>>>> Let me know what you think,
>>>> Jc
>>>>
>>>>
>>>>
>>>> --
>>>>
>>>> Thanks,
>>>> Jc
>>>
>>>
>>>
>>>
>>> --
>>>
>>> Thanks,
>>> Jc
>>
>>
>>
>>
>> --
>>
>> Thanks,
>> Jc
>
>
>
>
> --
>
> Thanks,
> Jc
--
Thanks,
Jc
