Hi Jc,

It looks good to me.
Thank you for taking care about it!

Thanks,
Serguei



On 9/14/18 9:50 AM, JC Beyler wrote:
Hi all,

Could I get a review for the following webrev:
Webrev: http://cr.openjdk.java.net/~jcbeyler/8210481/webrev.00/ <http://cr.openjdk.java.net/%7Ejcbeyler/8210481/webrev.00/>
Bug: https://bugs.openjdk.java.net/browse/JDK-8210481

@Alex: I know we had said generally 50 files max but this one is really straight forward; if you still prefer 50 files, let me know and I'll cut it up into chunks.

@all: A big webrev this time, to which I apologize but it is the last macro removal webrev from me in a while (there still is https://bugs.openjdk.java.net/browse/JDK-8210728 but I'm giving everyone a rest on macro removal and will work on a few code refactoring I promised to do ;-)).

To perhaps help reviewing it:
This webrev removes two #ifdef per file touched:

-#ifdef __cplusplus
 extern "C" {
-#endif

and

-#ifdef __cplusplus
 }
-#endif

-> The patch only has deletions (4 per file), no insertions
-> I double checked that all files only have exactly those removals and no other removals EXCEPT: http://cr.openjdk.java.net/~jcbeyler/8210481/webrev.00/test/hotspot/jtreg/vmTestbase/nsk/share/native/nsk_tools.h.udiff.html <http://cr.openjdk.java.net/%7Ejcbeyler/8210481/webrev.00/test/hotspot/jtreg/vmTestbase/nsk/share/native/nsk_tools.h.udiff.html> which is still defining the NSK_STUB macros but was doing it for C/C++ styles.
-> Last easier check for reviewers:
   - Look for deletions but that are not exactly "-#endif" or "-#ifdef __cplusplus" grep "^-[^-]" jdk12-jvmti12.patch | grep -v "^-#endif$" | grep -v "^-#ifdef __cplusplus$"

That command provides only the lines from the nsk_tools.h file:
$ grep "^-[^-]" jdk12-jvmti12.patch | grep -v "^-#endif$" | grep -v "^-#ifdef __cplusplus$"
-#define NSK_CPP_STUB1(Func,env) (*env)->Func(env)
-#define NSK_CPP_STUB2(Func,env,a) (*env)->Func(env,a)
-#define NSK_CPP_STUB3(Func,env,a,b) (*env)->Func(env,a,b)
-#define NSK_CPP_STUB4(Func,env,a,b,c) (*env)->Func(env,a,b,c)
-#define NSK_CPP_STUB5(Func,env,a,b,c,d) (*env)->Func(env,a,b,c,d)
-#define NSK_CPP_STUB6(Func,env,a,b,c,d,e) (*env)->Func(env,a,b,c,d,e)
-#define NSK_CPP_STUB7(Func,env,a,b,c,d,e,f) (*env)->Func(env,a,b,c,d,e,f)
-#define NSK_CPP_STUB8(Func,env,a,b,c,d,e,f,g) (*env)->Func(env,a,b,c,d,e,f,g) -#define NSK_CPP_STUB9(Func,env,a,b,c,d,e,f,g,h) (*env)->Func(env,a,b,c,d,e,f,g,h)
-#ifndef NSK_CPP_STUBS_ENFORCE_C
-#undef NSK_CPP_STUB1
-#undef NSK_CPP_STUB2
-#undef NSK_CPP_STUB3
-#undef NSK_CPP_STUB4
-#undef NSK_CPP_STUB5
-#undef NSK_CPP_STUB6
-#undef NSK_CPP_STUB7
-#undef NSK_CPP_STUB8
-#undef NSK_CPP_STUB9

Thanks for the reviews,
Jc

Reply via email to