RFR (M) 8210689: Remove the multi-line old C style for string literals

[JC Beyler]

Hi Chris,

Done here:
Webrev: http://cr.openjdk.java.net/~jcbeyler/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.plummer at 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/
> 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.plummer at 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.plummer at 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 <jcbeyler at google.com> wrote:
>>>
>>>> Hi all,
>>>>
>>>> Could I have a review for this webrev:
>>>>
>>>> Webrev: http://cr.openjdk.java.net/~jcbeyler/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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20180921/a8f5bf2c/attachment.html>