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

[Kurchi Hazra]

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