Hi JC,
Looks good.
For the bytecodes001.cpp formatting, I just thought it odd you put
a two char argument on the same line and the rest on the new line.
All on the same line seemed better, either as you have changed it
to or all on the next line.
thanks,
Chris
On 9/5/18 12:37 PM, JC Beyler wrote:
Hi Chris,
I had seen that formatting in bytecodes001.cpp but was
not sure it made the line a bit too long (I think there is
no real line wrap limit, just what seems reasonable,
correct?). But done since you saw it too, that makes two of
us wanting to change it!
I changed all the IsSameObject != JNI_TRUE in the files I
changed and added it to my refactoring logic for the next
round of refactoring.
Here is the new webrev with the metadata if you wanted to
see it (I was not sure if you wanted to see it again):
I double checked testing and it passes again all tests
changed on my linux dev machine.
Thanks again,
Jc
Hi JC,
In bytecodes001.cpp the formatting could use some
improvement:
130 if (meth_tab[meth_ind].stat == JNI_TRUE) {
131 mid = env->GetStaticMethodID(cl,
132 meth_tab[meth_ind].name,
meth_tab[meth_ind].sig);
133 } else {
134 mid = env->GetMethodID(cl,
135 meth_tab[meth_ind].name,
meth_tab[meth_ind].sig);
136 }
In getclsldr003.cpp, getfldecl001.cpp, getfldecl002.cpp,
and getfldecl004.cpp, no need to compare with JNI_TRUE.
Just negate the _expression_.:
98 if (env->IsSameObject(classloader, cl) !=
JNI_TRUE) {
Otherwise looks good.
Thanks for the cleanup,
Chris
On 9/4/18 4:12 PM, JC Beyler wrote:
Hi all,
Continuing the removal of the JNI_ENV*
macros, I have done about half of the JVMTI
Get[A-F] methods. The whole change for
jvmti/GetXX tests was about 2k of changes so it
seemed best to just divide it into two parts
(from my approximation there will be 2 more
webrevs to finish the umbrella item
JDK-8209547).
The change is straightforward as before,
just a bit repetitive:
I tested this in release mode for the tests
that were modified.
Thanks,
--
|