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

JC Beyler jcbeyler at google.com
Fri Sep 21 17:55:26 UTC 2018


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?


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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20180921/a200f0a3/attachment.html>


More information about the serviceability-dev mailing list