RFR (M) 8214892: Delayed starting of debugging via jcmd

[Chris Plummer]

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