On 4/30/19 4:30 AM, Schmelter, Ralf wrote:
Hi Chris.
I think print_debug_listen_address() should have some exception checking
added after the java calls.
That is already done by the CHECK_false macro. It checks if an exception is
occurred, and returns with false if this is the case. And depending on how the
dcmd was called, this exception is then handled (e.g. if called via jcmd, the
exception is printed and if called the jmm_ExecuteDiagnosticCommand method, the
exception is delivered to the Java code).
Ok.
I'm a little unsure why you modified DebugOnCmdStartDCmd to use
print_debug_listen_address(), but still have a fallback to print the
specified transport and address.
Yeah, the fallback is really not needed, since the debugger is not listening in
this case. I've removed it from the code.
If anything I would have written a
get_debug_listen_address() and used it to verify that the specified and
actual addresses end up being the same (and then also make
print_debug_listen_address() use this API).
Because the requested address and the actual address the debugger is listening
can be different. In the case of sockets, if you request localhost:0, the
system will find an unused port, so the listening address will be the actual
port on which we listen.
Ok.
You need to update copyright date to 2019.
Fixed.
Can you write a test for this new dcmd. You can probably just extend
OnJcmdTest.java.
The GetListenAddressTest tests the new dcmd.
Ok. I missed it because when after reviewing the OnJcmdTest.java changes
in "frames" mode and then clicking on "next" to review the next file, it
returns you to the index. Seems this is normal behavior for new files in
webrevs since there is no "frames" mode for them.
Changes look good.
Chris
I've updated the webrev (including David's suggestion):
http://cr.openjdk.java.net/~rschmelter/webrevs/8223065/webrev.2/
Best regards,
Ralf
