Code review request: 7146763: Warnings cleanup in the sun.rmi and related packages (original) (raw)

Kurchi Hazra kurchi.subhra.hazra at oracle.com
Wed Feb 22 21:25:27 UTC 2012

Hi Remi,

Thanks for your review. Please see my comment:

On 2/22/2012 10:01 AM, Rémi Forax wrote:

Hi Kurchi, hi all,

in ReliableLog, you can get ride of the @SupressWarnings, getLogClassConstructor should return a Constructor<?> and not a Constructor<? extends LogFile>, the field logClassConstructor should be typed Constructor<?> and in openLogFile, the log should be constructed like this log = (logClassConstructor == null ? new LogFile(logName, "rw") : (LogFile)logClassConstructor.newInstance(logName, "rw"));

The idea is that a cast on a LogFile is typesafe but not a cast on a Constructor<? extends LogFile>.

If I change the return type to Constructor<?>, I get the following error: ../../../../src/share/classes/sun/rmi/log/ReliableLog.java:122: error: incompatible types logClassConstructor = getLogClassConstructor(); ^ required: Constructor<? extends LogFile> found: Constructor<CAP#1> where CAP#1 is a fresh type-variable: CAP#1 extends Object from capture of ? And the following warning:

../../../../src/share/classes/sun/rmi/log/ReliableLog.java:350: warning: [unchecked] unchecked cast cl.getConstructor(String.class, String.class); ^ required: Constructor<? extends LogFile> found: Constructor<CAP#1> where CAP#1 is a fresh type-variable: CAP#1 extends Object from capture of ?

Thanks, Kurchi

In the same file, the try-with-resources is not correcly used try (DataOutputStream out = new DataOutputStream(new FileOutputStream(fName(name)))) { writeInt(out, version); } should be try (FileOutputStream fos = new FileOutputStream(fName(name)); DataOutputStream out = new DataOutputStream(fos)) { writeInt(out, version); } because if new DataOutputStream throws an exception, the FileOutputStream will not be closed. Basically, all stream filters should have it's own line in a try-with-resources. cheers, Rémi On 02/22/2012 12:16 PM, Chris Hegarty wrote: Kurchi,

Great work. I've been through the complete webrev and I think it looks good. Just a few minor comments: - there are API changes, but only in sun private implementation classes, so this should be fine. - Minor indentation nit where method declaration return type was generified. The method args on subsequent lines should be equally indented. Example LoaderHandler.java L152: public static Class<?> loadClass(String codebase, String name, >>>> ClassLoader defaultLoader) - There are opportunities to use auto boxing/unboxing >: diff RMIGenerator.java 99c99 < version = versionOptions.get(arg);_ _---_ _> version = (versionOptions.get(arg)).intValue(); ConnectionMultiplexer.java >: diff ConnectionMultiplexer.java ConnectionMultiplexer.java.1 133a134 < Integer idObj;_ _150c151,152_ _> info = connectionTable.get(id); --- < idObj = new Integer(id);_ _< info = connectionTable.get(idObj);_ _158c160_ _> connectionTable.put(id, info); --- < connectionTable.put(idObj, info); ..... -Chris. On 22/02/2012 05:50, Kurchi Hazra wrote: Corrected the subject line.

Hi, The following webrev removes warnings in sun.rmi.* packages. I have neglected nearly all deprecation warnings, since this code uses deprecated classes such as java.rmi.server.LogStream with no suggested replacements. I have included -Xlint:all,-deprecation as an option instead in the appropriate Makefiles. Bug: http://bugs.sun.com/bugdatabase/viewbug.do?bugid=7146763 Webrev: http://cr.openjdk.java.net/~khazra/7146763/webrev.00/ Thanks, Kurchi

-- -Kurchi

More information about the core-libs-dev mailing list