Hi Serguei, I have https://bugs.openjdk.java.net/browse/JDK-8212960 open for that. I like doing things in small contained steps for reviewing and sanity checking. But if you want, I can run a script to fix all these cases now and resent a RFR. I created https://bugs.openjdk.java.net/browse/JDK-8213499 for the arguments.
Let me know what you prefer, I can see the pros/cons of both ways :-), Jc On Wed, Nov 7, 2018 at 1:01 PM [email protected] < [email protected]> wrote: > Hi Jc, > > +1 LGTM. > > One question: > Would it be convenient at the same time to fix missing spaces around > commas and comparisons in loops and conditions. > One more case is function call arguments but I did not see any instances > of it. > > Some examples: > > > http://cr.openjdk.java.net/%7Ejcbeyler/8212939/webrev.00/test/hotspot/jtreg/vmTestbase/nsk/jvmti/Breakpoint/breakpoint001/breakpoint001.cpp.udiff.html > > - for(i=0; i<METH_NUM; i++)+ for (i=0; i<METH_NUM; i++) > > > > http://cr.openjdk.java.net/%7Ejcbeyler/8212939/webrev.00/test/hotspot/jtreg/vmTestbase/nsk/jvmti/ClassLoad/classload001/classload001.cpp.udiff.html > > - for(i=0; i<EXP_SIG_NUM; i++)+ for (i=0; i<EXP_SIG_NUM; i++) > clsEvents[i] = 0; > - for(i=0; i<UNEXP_SIG_NUM; i++)+ for (i=0; i<UNEXP_SIG_NUM; i++) > > > > > http://cr.openjdk.java.net/%7Ejcbeyler/8212939/webrev.00/test/hotspot/jtreg/vmTestbase/nsk/stress/jni/libjnistress004.cpp.udiff.html > > There are many cases like below: > > - for(i=0;i<DIGESTLENGTH;i++) {+ for (i=0;i<DIGESTLENGTH;i++) { > > . . . > > - for(i=0;i<len;i++)- if(ch[i]!=tmp[i]) {+ for (i=0;i<len;i++)+ > if (ch[i]!=tmp[i]) { > > > Thanks, > Serguei > > > On 11/7/18 12:23, Chris Plummer wrote: > > Hi JC. > > In Callbacks.cpp you missed the needed extra indent on the lines following > the "if": > > 406 if (!NSK_JVMTI_VERIFY(jvmti->GetFieldName(targetClass, > 407 targetFields[field], > 408 &objects_info[object].fields[field].name, > 409 &objects_info[object].fields[field].signature, > 410 NULL))) > > 481 if ((objects_info[object].fields[field].primitive && > !objects_info[object].collected) > 482 || !objects_info[object].fields[field].collected) { > > Otherwise looks good. I don't need to see another webrev. > > thanks, > > Chris > > On 11/7/18 11:20 AM, JC Beyler wrote: > > Hi all, > > Could I get a review for a relatively straight-forward space > transformation webrev? This adds spaces between keywords and '(' and fixes > the case "){" across C++ files in vmTestbase. > > Webrev: http://cr.openjdk.java.net/~jcbeyler/8212939/webrev.00/ > <http://cr.openjdk.java.net/%7Ejcbeyler/8212939/webrev.00/> > <http://cr.openjdk.java.net/%7Ejcbeyler/8212939/webrev.00/> > Bug: https://bugs.openjdk.java.net/browse/JDK-8212939 > > Thanks! > Jc > > > > > > -- Thanks, Jc
