Code Review Request for 6578042

Alan Bateman Alan.Bateman at oracle.com
Wed Nov 2 09:47:50 UTC 2011


On 01/11/2011 23:54, David Holmes wrote:
>
> This fix seems inconsistent with how this non-String problem is 
> handled elsewhere. 
It is, but (for whatever reason) is specified to return "the previous 
string value of the system property" whereas Properties.getProperty is 
specified to return "the value in this property list with the specified 
key value". Given that System.clearProperty has always been broken then 
there may be an opportunity to change it so that it is consistent and 
returns null.  I also think Properties.getProperty could be clearer as 
it doesn't appear to specify that it returns null when the value isn't 
of type String.

Darryl - just a couple of comments on the test - as it doesn't clean up 
after itself then it will need to run in its own VM to ensure that it 
doesn't cause problems for other tests that run subsequently in the same 
VM (@run main/othervm ClearProperty). Another nit with the test is that 
it doesn't check the return value of System.clearProperty. Another 
comment is that it doesn't need to catch CCE as the test will fail 
anyway if thrown. Also it would be nice to separate the test comment 
from the comment block with the jtreg tags so that it's consistent with 
other tests (alternatively just expand the wording in the @summary tag).

-Alan.



More information about the core-libs-dev mailing list