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

Joe Darcy joe.darcy at oracle.com
Wed Aug 29 23:56:46 UTC 2012


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.FILEREADACTION: 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.FILEREADACTION) { 437 return READ; 438 } else if (actions == SecurityConstants.FILEWRITEACTION) { 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



More information about the core-libs-dev mailing list