Code review request: 7146763: Warnings cleanup in the sun.rmi and related packages

Kurchi Hazra kurchi.subhra.hazra at oracle.com
Thu Feb 23 21:32:14 UTC 2012

I have included all changes/suggestion except the Constructor modification that Remi suggested.

Darryl:

TCPTransport: Lines #552/553: I actually prefer the way it was as I think it's more readable.

RemoteCall is deprecated and I was trying to avoid creating an instance there to remove the resultant Deprecation warning - but I reverted this back

RMIMasterSocketFactory: Line #244: The comment was removed, was there a reason for this? This was a mistake. Changed it now.
Kurchi

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>. >> 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