Re: RFR (L) 8220747: Migrate data structures to being more C++

2019-05-01 Thread David Holmes

Hi Jc,

I just a had a quick look at this so not a full review - sorry.

I'm not sure it makes sense to define classes within "extern C {". The 
extern C is intended to define an interface for this C++ library to be 
used from a C program - as discussed here for your Solaris issue:


https://docs.oracle.com/cd/E18659_01/html/821-1383/bkamu.html

Cheers,
David

On 2/05/2019 1:08 pm, Jean Christophe Beyler wrote:

Hi all,

Re-sending with the full title

(OK... so JC will promise to go around the block 3 times before submitting
a review request; and will do any item you would like to redeem myself; I
apologize profusely and feel horrible...)

I want to move the libHeapMonitorTest.c to C++ and here is the first "step"
towards that. There are two parts to this: move the file to C++ and move
some of the C-style to C++-style code.

But this webrev failed on solaris; Igor helped me figure it out and his
solution was to add the change to the JtregNativeHotspot.gmk for solstudio.
We are not sure this is the "right" solution to this and hence have added
both the serviceability and build lists to review both file changes and
figure out what is best :)

This does pass the submit-repo testing and the tests on my local dev
machine.

Bug: https://bugs.openjdk.java.net/browse/JDK-8220747
Webrev: http://cr.openjdk.java.net/~jcbeyler/8220747/webrev.02/

Thanks all for your help,
Jc



RFR (L) 8220747: Migrate data structures to being more C++

2019-05-01 Thread Jean Christophe Beyler
Hi all,

Re-sending with the full title

(OK... so JC will promise to go around the block 3 times before submitting
a review request; and will do any item you would like to redeem myself; I
apologize profusely and feel horrible...)

I want to move the libHeapMonitorTest.c to C++ and here is the first "step"
towards that. There are two parts to this: move the file to C++ and move
some of the C-style to C++-style code.

But this webrev failed on solaris; Igor helped me figure it out and his
solution was to add the change to the JtregNativeHotspot.gmk for solstudio.
We are not sure this is the "right" solution to this and hence have added
both the serviceability and build lists to review both file changes and
figure out what is best :)

This does pass the submit-repo testing and the tests on my local dev
machine.

Bug: https://bugs.openjdk.java.net/browse/JDK-8220747
Webrev: http://cr.openjdk.java.net/~jcbeyler/8220747/webrev.02/

Thanks all for your help,
Jc