review of simple pack200 fix

David Holmes David.Holmes at oracle.com
Thu Dec 16 05:07:05 UTC 2010


Hi Mike,

Mike Duigou said the following on 12/16/10 09:10:
> Under what circumstances is null returned and why is it safe to ignore it? Is the handling of null related to the lack of a default label on the switch? ie. It is intentional that action is only taken with the 3 specified commands and null is just in the set of cmd values requiring no special action.
> 
> It makes me nervous whenever I see "pretend this never happened" handling for null.

You need to know the history here. Originally the code was:

+            if (attrCommands != null) {
+                Object lkey = Attribute.keyForLookup(ctype, name);
+                String cmd = (String) attrCommands.get(lkey);
+                if (cmd == "pass") {
+                    String message = "passing attribute bitwise in "+h;
+                    throw new Attribute.FormatException(message, ctype, 
name,
+                                                        cmd);
+                } else if (cmd == "error") {
+                    String message = "attribute not allowed in "+h;
+                    throw new Attribute.FormatException(message, ctype, 
name,
+                                                        cmd);
+                } else if (cmd == "strip") {
+                    skip(length, name+" attribute in "+h);
+                    continue;
+                }
+            }

then at some point (which doesn't seem to have filtered through to the 
repo I clone) the if/else was changed to the switch, but the switch 
didn't account for the possibility there may not be a cmd for a 
particular case.

Cheers,
David

> Mike
> 
> On Dec 15 2010, at 14:45 , Kumar Srinivasan wrote:
> 
>> Hi,
>>
>> Could you please review this simple  fix, it guards the switch value from a null.
>>
>> http://cr.openjdk.java.net/~ksrini/7007157/webrev.00/
>>
>> Thanks
>> Kumar
>>
>>
>>
> 



More information about the core-libs-dev mailing list