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

Jean Christophe Beyler jcbeyler at google.com
Wed May 8 23:06:13 UTC 2019


Hi David,

Thanks for the quick look :)

I moved the code around to really only have the minimal part in the extern
C part. It seems that the link you are providing seems to show that what we
did is the right thing then so that seems like it's a good start I guess!

Here is an updated webrev:
Webrev: http://cr.openjdk.java.net/~jcbeyler/8220747/webrev.03/

I left the structs in structs so I did not have to append the fields with
'_', which otherwise made the code review feel (to me) messy.

Thanks again for your help,
Jc

On Wed, May 1, 2019 at 10:22 PM David Holmes <david.holmes at oracle.com>
wrote:

> 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
> >
>


-- 

Thanks,
Jc



More information about the build-dev mailing list