Code Review Request: 7117570: Warnings in sun.mangement.* and its subpackages

[Dmitry Samersoff]

On 2012-01-13 02:30, Kurchi Hazra wrote:

>> 1. Agent.java
>>
>> 220        comments is not relevant any more.
>>
>> 248        check of existence of configFile is not necessary.
> 
> Thanks, I removed these.
> 
>> 255-258 a) Exceptions could be collapsed
> 
> This cannot be collapsed since FileNotFoundException is a subclass of
> IOException.

Does it mean that we can simple remove catch of FileNotFoundException,
replacing it with a comments that FileNotFound case covered by IOException ?

 263 } catch (FileNotFoundException e) {
 264             error(CONFIG_FILE_OPEN_FAILED, e.getMessage());
 265 } catch (IOException e) {
 266             error(CONFIG_FILE_OPEN_FAILED, e.getMessage());

This code looks really weird for me, could you at least add a comment?


> 
>>          b) finally is gone - is it expected?
>>              CONFIG_FILE_CLOSE_FAILED is never happens anymore.
>>
>>              I would prefer to keep original code
> 
> - We were trying to use try-with-resources (new JDK7 feature), where
> closing of resources is automatically taken care of. But I will
> revert back to original code if that is preferred.

We going to an area of changing public symbols. I'm in doubt that
someone rely on CONFIG_FILE_CLOSE_FAILED message, but we need to check
it, remove it from all translations etc.

IMHO, It's out of the scope of your amazing work.


>>
>> 2. ConnectorAddressLink.java
>>     176 Not sure implicit toString() necessary here.
> I am not sure too. getValue() returns an Object.

OK.

Thank you!
-Dmitry



-- 
Dmitry Samersoff
Java Hotspot development team, SPB04
* There will come soft rains ...