RFR 8039080: "jinfo server_id at host" fails with "Invalid process identifier"

[Jaroslav Bachorik]

On 3.4.2014 14:33, Staffan Larsen wrote:
>
> On 3 apr 2014, at 14:20, Jaroslav Bachorik <jaroslav.bachorik at oracle.com> wrote:
>
>> On 3.4.2014 14:08, Dmitry Samersoff wrote:
>>> Jaroslav,
>>>
>>> Command line processing is still not clean, e.g. you check for -help in
>>> three different places.
>>>
>>> If you go to refactor this code rather than provide a minimal fix,
>>> it might be better to create a separate inner class that do all command
>>> line processing and provide couple of flags like should_sa(),
>>> should_help(), should_attach() etc.
>>
>> Yes, I know :(
>>
>> My first approach was exactly like that. But I was rewriting the command line parser from scratch for at least the 3rd time for "tools" alone :( And the change was going to be rather big - more of a complete rewrite than a fix.
>
> I just re-wrote that command line parser. Obviously with mixed results… :(  I think the real problem here is that this command line is not specified in a way that is easily parseable, and we don’t want to change how it works.
>
> Command line parsing is one of the really hard areas in computer science.

There are some rather flexible frameworks allowing nice command line 
parsing in java. But they are all 3rd party utilities.

>
>
> You have added two methods that do not look like they are being called: useSA() and getArgs().

These two methods are there only to make the code testable without 
resorting to reflection and setAccessible(). Not sure what is the lesser 
evil ...

>
> Otherwise the changes look good.
>
> If you haven’t already, make sure sun/tools/jinfo/Basic.sh passes on all platforms and add cases to it if that is appropriate. (Note that it currently fails on solaris JDK-8038296).
Basic.sh should not be affected by this change at all. But I will make 
sure the tests pass on all platforms before pushing.

-JB-

>
> Thanks,
> /Staffan
>