Review Request: 7193406 - Clean-up JDK Build Warnings in java.util,

Stuart Marks stuart.marks at
Thu Aug 30 02:07:19 UTC 2012

On 8/29/12 4:56 PM, Joe Darcy wrote:
> On 8/29/2012 4:20 PM, Stuart Marks wrote:
>> The original code was like this:
>>  427     private static int getMask(String actions) {
>>              ...
>>  435         // Check against use of constants (used heavily within the JDK)
>>  436         if (actions == SecurityConstants.FILE_READ_ACTION) {
>>  437             return READ;
>>  438         } else if (actions == SecurityConstants.FILE_WRITE_ACTION) {
>> The various SecurityConstants being used here are Strings.
>> Note that this is doing String comparisons using == which is usually a bug.
>> But this code is relying on String interning to provide object identity for
>> equal string literals, in order to provide a fast path optimization for these
>> common cases. Changing this code to use switch-over-strings would be
>> semantically equivalent, but it would probably slow things down, since switch
>> provides equals() semantics instead of == semantics.
> This would be a fine point to highlight in the comments around this check!

The comment at line 435 did serve to tip me off that something unusual was 
going on with this code, so it served its purpose. But I'll admit that it's not 
as explicit as it could be. How about this:

     // Use object identity comparisons with String constants for performance
     // reasons (these values are used heavily within the JDK).


More information about the core-libs-dev mailing list