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

Ulf Zibis Ulf.Zibis at CoSoCo.de
Wed Aug 29 10:50:02 UTC 2012

Am 24.08.2012 02:12, schrieb Andrew Hughes:

In FilePermission.java file, I make one change to its method signature,

public Enumeration elements() ==> public Enumeration elements()

I am not sure whether it will cause an issue of backward compatibility. Please advise. Thanks! Actually the whole method is synchronized. To make this more clear, I suggest: 798 public synchronized Enumeration elements() { 799 // Convert Iterator into Enumeration 800 return Collections.enumeration(perms); 801 }

It's in a package-private class so I doubt it.

Even if it wasn't, a new major release is the perfect time to fix these issues.

If this class is package-private, why the constructor is public? (please also check all other methods, especially if not inherited)

Please check indentations and tabs -> spaces, while you are here. E.g. following lines should be: 713 final class FilePermissionCollection extends PermissionCollection 714 implements Serializable {

732 * @param permission the Permission object to add. 733 * 734 * @throws IllegalArgumentException - if the permission is not a

743 if (! (permission instanceof FilePermission)) 744 throw new IllegalArgumentException( 745 "invalid permission: "+permission); 746 if (isReadOnly()) 747 throw new SecurityException( 748 "attempt to add a Permission to a readonly PermissionCollection");

767 if (! (permission instanceof FilePermission)) 768 return false;

Lines 822..825 look ugly. Better: 819 /** 820 * @serialData "permissions" field (a Vector containing the FilePermissions). 821 / 822 / 823 Writes the contents of the perms field out as a Vector for 824 serialization compatibility with earlier releases. 825 / or: 819 /* 820 * @serialData "permissions" field (a Vector containing the FilePermissions). 821 */ 822 // Writes the contents of the perms field out as a Vector for 823 // serialization compatibility with earlier releases.

I only took a short look on class FilePermission.java, but not the others of this CR for now.

-Ulf

More information about the core-libs-dev mailing list