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

[Joe Darcy]

On 8/29/2012 4:20 PM, Stuart Marks wrote:
> On 8/29/12 4:36 AM, Ulf Zibis wrote:
>>>>  436         switch (actions) {
>>>>  437             case SecurityConstants.FILE_READ_ACTION:
>>>>  438                 return READ;
>>
>> Oops, you have reverted the change to use switch-on-Strings by 
>> webrev.03. Why?
>
> A fair question. I had instigated some internal conversation about 
> reverting this change, so here's the explanation. (Good catch, by the 
> way.)
>
> 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!

-Joe