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/view_bug.do?bug_id=7146763
Webrev: http://cr.openjdk.java.net/~khazra/7146763/webrev.00/
Thanks,
Kurchi
--
-Kurchi
