Hi Dmitry, > Otherwise looks good for me (I'll sponsor the push).
Thanks! > 1. It's better to check _libpath.value() and write a message, that the > parameter is required. I set true to 4th argument (mandatory) of DCmdArgument c'tor. If we execute jcmd without libpath, we get message as below: ----------- $ jcmd 939 JVMTI.javaagent_load 939: java.lang.IllegalArgumentException: The argument 'library path' is mandatory. ----------- > 2. It might be better to avoid malloc here by pre-calcualting length of > required option length and than declare > > char *new_options[new_options_len]; Sorry, I could not understand details. We have to be different way for option string of libinstrument. If option value is not NULL, we have to build string like <libpath>=<option> as argument of libinstrument. Otherwise, we can pass libpath only to libinstrument. I use malloc() and strdup() to build option string to pass to libinstrument.so . Thanks, Yasumasa On 2016/01/15 0:06, Dmitry Samersoff wrote: > Yasumasa, > > diagnosticCommand.cpp:294 > > 1. It's better to check _libpath.value() and write a message, that the > parameter is required. > > 2. It might be better to avoid malloc here by pre-calcualting length of > required option length and than declare > > char *new_options[new_options_len]; > > Otherwise looks good for me (I'll sponsor the push). > > -Dmitry > > > On 2016-01-14 17:00, Yasumasa Suenaga wrote: >> Hi all, >> >> We can use Attach API to attach JVMTI agent to live process. >> However, we have to write Java code for it. >> >> If we can attach JVMTI agents through jcmd, it is very useful. >> So I want to add two new diagnostic commands: >> >> * JVMTI.agent_load: Load JVMTI native agent. >> * JVMTI.javaagent_load: Load JVMTI java agent. >> >> I maintain two JVMTI agents - HeapStats [1] and JLivePatcher [2]. >> [1] is native agent, [2] is java agent. >> They provide a program for attaching to live process. >> >> I guess that various JVMTI agents provide own attach mechanism like them. >> I think that we should provide general way to attach. >> >> I've uploaded webrev. Could you review it? >> http://cr.openjdk.java.net/~ysuenaga/JDK-8147388/webrev.00/ >> >> I'm jdk9 committer, however I cannot access JPRT. >> So I need a sponsor. >> >> >> Thanks, >> >> Yasumasa >> >> >> [1] http://icedtea.classpath.org/wiki/HeapStats >> [2] https://github.com/YaSuenag/jlivepatcher (in Japanese) >> > >
