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

[Rémi Forax]

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