RFR 8223065: Add jcmd to get the listen address of the debugger
Langer, Christoph
christoph.langer at sap.com
Tue May 7 12:36:32 UTC 2019
Hi Ralf,
the change looks good to me, overall. I found a few minor nits in the tests.
test/jdk/com/sun/jdi/OnJcmdTest.java
3 * Copyright (c) 2018, 2019, SAP SE. All rights reserved.
-> according to our guidelines, we should not have a ',' after the last year for the SAP copyrights
33 * @run compile --add-exports java.base/jdk.internal.vm=ALL-UNNAMED -g OnJcmdTest.java
-> can you replace this with @modules java.base/jdk.internal.vm ?
The -g option is also not required, I guess (unless somebody debugs the test)
Then you should be able to change
34 * @run main/othervm --add-exports java.base/jdk.internal.vm=ALL-UNNAMED -agentlib:jdwp=transport=dt_socket,address=localhost:0,onjcmd=y,server=y OnJcmdTest
into
@run main/othervm -agentlib:jdwp=transport=dt_socket,address=localhost:0,onjcmd=y,server=y OnJcmdTest
37 import java.lang.reflect.Method;
-> can be removed
test/jdk/com/sun/jdi/GetListenAddressTest.java
-> SAP copyright header needs fixing like above
-> 33 * @run compile -g GetListenAddressTest.java should be removed
-> 37 import java.lang.ProcessBuilder.Redirect; -> is unnecessary, should be removed
I'll sponsor the change for you.
Best regards
Christoph
> -----Original Message-----
> From: serviceability-dev <serviceability-dev-bounces at openjdk.java.net> On
> Behalf Of Baesken, Matthias
> Sent: Montag, 6. Mai 2019 12:55
> To: serviceability-dev at openjdk.java.net
> Subject: [CAUTION] Re: RFR 8223065: Add jcmd to get the listen address of
> the debugger
>
> Hello, looked at the latest web rev (
> http://cr.openjdk.java.net/~rschmelter/webrevs/8223065/webrev.3/ ) ,
> looks good to me !
> (not a Reviewer however )
>
>
> Best regards, Matthias
>
>
> >
> > On 30/04/2019 9:33 pm, Schmelter, Ralf wrote:
> > > Hi David,
> > >
> > > good catch. I've moved the vm->native transition back to the start of the
> > method and instead do a native->vm transition before calling
> > print_debug_listen_address() method.
> > >
> > > webrev:
> > http://cr.openjdk.java.net/~rschmelter/webrevs/8223065/webrev.2/
> >
> > Yep that works too. :)
> >
> > Not a review as I didn't look at the rest of the code.
> >
> > Cheers,
> > David
> >
> > > Best regards,
> > > Ralf
> > >
> >
> >
More information about the serviceability-dev
mailing list