Hi Ralf,
I don't think the comments from Goetz made it to the list. However, with
the current changes it all looks good to me, except there are some
indentation issues. Indent tab stops are should be increments of 2 for
hotspot code. You have a few cases of using 4 in the dcmd files. For the
jdwp agent, pre-existing code uses tab stops of 4, so should remain that
way, and it looks like you've correctly done this.
thanks,
Chris
On 12/6/18 6:54 AM, Schmelter, Ralf wrote:
Hi Chris,
I think I prefer passing EI_VM_INIT for triggering_ei, but if you prefer
to keep it as is, I suggest a comment to clarify why it might be out of
range.
Actually, I used EI_VM_INIT for the longest time and only changed it recently,
because I thought that code could assume that e.g. no classes have been loaded
yet when getting the INIT_VM event. But since the JVMTI spec does not
guarantees this in any way (it allows other events to be send before a
VM_INIT), I just will change it back to use EI_VM_INIT for the initialize call.
Regarding the name of the option, I agree that onjcmd, while not technically
fully accurate, makes most sense for the common user.
... It think you could just return the error right away and remove
the error checking code that comes later.
I've changed the code to just return the error directly.
It's not clear to me why you want the name and address of the first
transport. Why is the first one necessarily the one you want?
Since currently the bag of transports must always have a size of 1, getting the
first or the last transport is the same. But the callback function used to
iterate the bag has to return a boolean value. I've decided to return
JNI_FALSE, which would mean the iteration stops at the first entry.
I've updated the webrev with your and Goetz Lindenmeier's suggestions:
http://cr.openjdk.java.net/~rrich/webrevs/schmelter/8214892/webrev.02/
Best regards,
Ralf
