RFR (S) 8025638: jmap returns 0 instead of 1 when it fails.

[Kevin Walls]

Hi,

Yes the removal of System.exit didn't take the error value into 
account.  8010278 removed the System.exit so these Tool classes were 
usable by other tools.  I suppose we can use it in main() as if we're 
there we should be running as a standalone app.

The execute method seems like a nice simplification for the Tools.  An 
external app that wants to call one of the Tools can't use it, as it may 
call System.exit (and you don't want to make execute return an int as 
then every Tool class has to have more logic in its main() ).

nits in Tool.java
115 {  should be on the same line as execute definition begins.
120 } finally { on one line

I don't think stop() should become private, an existing app that used 
"public void start()" would expect to call a public stop().

There are a few other files that might want looking at to get them 
returning an error code: CLHSDB and HSDB (CommandProcessor looks OK, it 
only had System.exit(0) removed.)  That could be a separate bug...

Thanks!
Kevin


On 10/10/13 12:49, Staffan Larsen wrote:
> Thanks for doing this!
>
> Tool.java:116 - shouldn't the default return value be 1? In case 
> start() throws an exception for some reason.
>
> Tool.java: I find the start(String[] args), start(), startInternal() 
> methods confusing in naming and usage. Not directly related to your 
> change of course, just a comment on the code.
>
> /Staffan
>
>
> On 10 okt 2013, at 13:38, Staffan Larsen <staffan.larsen at oracle.com 
> <mailto:staffan.larsen at oracle.com>> wrote:
>
>> Clicking on the link in this email takes me to the wrong webrev. The 
>> correct URL is in the text: 
>> http://cr.openjdk.java.net/~allwin/farvidss/8025638/webrev.00/ 
>> <http://cr.openjdk.java.net/%7Eallwin/farvidss/8025638/webrev.00/>
>>
>> /Staffan
>>
>> On 10 okt 2013, at 12:42, Fredrik Arvidsson 
>> <fredrik.arvidsson at oracle.com <mailto:fredrik.arvidsson at oracle.com>> 
>> wrote:
>>
>>> Hi
>>>
>>> Please help me to review the changes below:
>>>
>>> Jira case: https://bugs.openjdk.java.net/browse/JDK-8025638 
>>> <https://bugs.openjdk.java.net/browse/JDK-8024423>
>>> Webrev: 
>>> http://cr.openjdk.java.net/~allwin/farvidss/8025638/webrev.00/ 
>>> <http://cr.openjdk.java.net/%7Eallwin/farvidss/8024423/webrev.00/>
>>>
>>> About this change.
>>> A previous change (https://bugs.openjdk.java.net/browse/JDK-8010278) 
>>> in the Tool.java class caused any tool deriving from this base class 
>>> return the wrong value to the caller when failing.
>>> Changes were made to the Tool.java class and to the derived tool 
>>> implementation classes to handle errors/exceptions during execution 
>>> and ensure that the tool returns 1 to the caller if it fails, and 0 
>>> if it succeeds.
>>>
>>> Previously failed Aurora tests have been run using UTE and verified 
>>> to PASS.
>>>
>>> Cheers
>>> /Fredrik
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20131010/6e6a0c65/attachment.html