RFR (L) 8212770: Remove spaces before/after () for vmTestbase/jvmti/[s-u]

[JC Beyler]

Thanks Chris,

Pushed then!
Jc

On Wed, Oct 24, 2018 at 9:32 PM Chris Plummer <chris.plummer at oracle.com>
wrote:

> Hi JC,
>
> webrev.00 is fine since you have bugs filed for the rest.
>
> thanks,
>
> Chris
>
> On 10/24/18 7:54 PM, JC Beyler wrote:
>
> Hi Chris,
>
> I inlined my answers:
>
> On Wed, Oct 24, 2018 at 2:58 PM Chris Plummer <chris.plummer at oracle.com>
> wrote:
>
>> Hi JC,
>>
>> Overall looks pretty good:
>>
>> I see some cases of changes on lines where there is an assignment in a
>> conditional. Will these conflict with your other webrev?
>>
>>      if (!NSK_JNI_VERIFY(jni, (root =
>> -                jni->GetStaticObjectField(debugeeClass, rootFieldID)) !=
>> NULL )) {
>> +                jni->GetStaticObjectField(debugeeClass, rootFieldID)) !=
>> NULL)) {
>>
>>
> Yes they might, I was going to go with the first that goes through and
> send an incremental for the other one. To be honest, I kind of was
> expecting perhaps some more conversation about extracting the assignments
> that I'd have time to fix the conflicts without anyone really noticing :)
>
>
>> I noticed a number of cases of checking if a boolean result is true or
>> false. I brought this up once before. I forgot if you filed a separate CR
>> for it:
>>
>> -        if (nsk_jvmti_parseOptions(options) == NSK_FALSE ) {
>> +        if (nsk_jvmti_parseOptions(options) == NSK_FALSE) {
>>
>>
> I can't find it so I created:
> https://bugs.openjdk.java.net/browse/JDK-8212959
>
>
>
>> The following is missing a space. This piece of code is probably
>> replicated at least a dozen times.
>>
>> -    if ( rc!= JNI_OK ) {
>> +    if (rc!= JNI_OK) {
>>
>> And a missing space here also. Only saw it in one place.
>>
>> -    if ( (strcmp(name, CLASS_NAME) ==0 ) && (redefineNumber == 0) ) {
>> +    if ((strcmp(name, CLASS_NAME) ==0) && (redefineNumber == 0)) {
>>
>> There's a double space here:
>>
>> -        if ( nsk_jvmti_redefineClass(jvmti_env, klass, fileName)  ==
>> NSK_TRUE ) {
>> +        if (nsk_jvmti_redefineClass(jvmti_env, klass, fileName)  ==
>> NSK_TRUE) {
>>
>>
> For all of these, I created:
> https://bugs.openjdk.java.net/browse/JDK-8212960
>
> Let me know if these answer your issues and if therefore  I can push
> webrev.00 in your opinion. Then I'll work on the potential conflicts for
> the other webrev,
> Jc
>
> thanks,
>>
>> Chris
>>
>> On 10/24/18 1:00 PM, JC Beyler wrote:
>>
>> Hi all,
>>
>> Anybody else want to review this? We can close the book on spaces
>> before/after () once this one goes in :)
>> Jc
>>
>> On Mon, Oct 22, 2018 at 1:37 PM Alex Menkov <alexey.menkov at oracle.com>
>> wrote:
>>
>>> LGTM.
>>>
>>> --alex
>>>
>>> On 10/22/2018 11:29, JC Beyler wrote:
>>> > Hi all,
>>> >
>>> > Here is the next webrev (second out of 3) to remove the spaces
>>> > after/before () from vmTestbase. It is straightforward and I generated
>>> > the webrev with white-space changes showing up of course:
>>> >
>>> > Webrev: http://cr.openjdk.java.net/~jcbeyler/8212770/webrev.00/
>>> > <http://cr.openjdk.java.net/%7Ejcbeyler/8212770/webrev.00/>
>>> > Bug: https://bugs.openjdk.java.net/browse/JDK-8212770
>>> >
>>> > Could I please get some reviews?
>>> >
>>> > 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/20181025/49e97fa9/attachment.html>