On Tue, 2 Mar 2021 08:30:06 GMT, Yasumasa Suenaga <ysuen...@openjdk.org> wrote:
>> `jhsdb` has `--pid`, `--core`, and `--connect` as the 3 ways to "attach" for >> lack of a better word. It would have been nice if the clhsdb command did a >> better job of copying these three command line arguments. You are suggesting >> that adding a `connect` command would have similarity to the `--connect` >> option, but unfortunately the `attach` command has no such similarity with >> the command line arguments `--pid` and `--core`, so I'm not so sure doing so >> with `connect` is actually helping in that regard, especially when "connect" >> and "attach" pretty much mean the same thing. >> >> As for numeric host names, yes, that is possible, but then you could also >> have a numeric core file name. If we really want to avoid the numeric host >> name problem, perhaps something like `attachd` would be better than >> `connect`, but it seems any choice we make will have it's drawbacks due the >> the baggage of existing commands and options. > >> @plummercj Ok, I try to add debug server support to `attach` clhsdb command. >> Then should we still need CSR? > > Pushed new commit to be implemented as `attach`. It works fine with > serviceability/sa jtreg tests on my Linux x64. > I will add CSR if you are ok. Hi, I ran the first version and agree it works, the connect command worked OK when I ran a local, manual test. But I do like adding to the attach Command rather than connect, avoiding introducing a new Command name. Assuming an int is a PID like in Tool.java is reasonable. You can have a leading numeral since a certain RFC, but although I can't find clear language to say it's definitely illegal, actualy using a fully numeric hostname seems unwise. CLHSDB.java line 185 Now that "private void attachDebugger(int pid)" takes an int, we don't want to catch NumberFormatException, that try/catch block is not wanted. In the testcase, I think it will fail if a command throws an Exception, but that is limiting what problems it can detect: could we make the test check at least some of the key output to show that we are attached and getting information, not errors or something unexpected? If at least some key commands we issue are checked that they contain some output indicating success that would be great. ------------- PR: https://git.openjdk.java.net/jdk/pull/2773
