RFR of JDK-8171133: java/rmi/registry/reexport/Reexport.java, there is a missing case check in createReg(..)

[Hamlin Li]

Hi Roger,

Thank you for reviewing, removed the redundant checking and pushed the code.

-Hamlin


On 2016/12/16 22:13, Roger Riggs wrote:
> Hi Hamlin,
>
> Yes, the logic would be clearer;  and I would also remove the (now) 
> redundant checking.
>
> Roger
>
>
> On 12/15/2016 9:42 PM, Hamlin Li wrote:
>> Hi Roger, Daniel,
>>
>> Thank you for reviewing.
>>
>> On 2016/12/15 22:45, Roger Riggs wrote:
>>> Hi Hamlin,
>>>
>>> If this is supposed to fix the call from line 68: then doesn't the 
>>> test for reg != null
>>> at line 70 already have the same effect?
>> Please consider the situation: LocateRegistry.createRegistry(port) in 
>> createReg(true, port) does not throw exception but just return null 
>> when remoteOk is true. this case is missed by current test. Although 
>> we know that should not happen by checking the product code, but the 
>> test should not assume how it is implemented.
>> And I think the patch will make logic more clear, maybe we could 
>> remove the check of reg != null at the same time after modify the 
>> code in createReg(...).
>>
>> Thank you
>> -Hamlin
>>>
>>> Roger
>>>
>>>
>>> On 12/14/2016 10:19 PM, Hamlin Li wrote:
>>>> Would you please review the below patch?
>>>>
>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8171133
>>>> webrev: http://cr.openjdk.java.net/~mli/8171133/webrev.00/
>>>>
>>>> java/rmi/registry/reexport/Reexport.java, there is a missing case 
>>>> check in createReg(..): if LocateRegistry.createRegistry(port) 
>>>> return null when port is in use.
>>>>
>>>> Thank you
>>>> -Hamlin
>>>> ------------------------------------------------------------------------ 
>>>>
>>>> diff -r ddd192238fcb test/java/rmi/registry/reexport/Reexport.java
>>>> --- a/test/java/rmi/registry/reexport/Reexport.java    Tue Dec 13 
>>>> 18:47:23 2016 -0800
>>>> +++ b/test/java/rmi/registry/reexport/Reexport.java    Wed Dec 14 
>>>> 19:06:40 2016 -0800
>>>> @@ -105,6 +105,9 @@
>>>>
>>>>          try {
>>>>              reg = LocateRegistry.createRegistry(port);
>>>> +            if (remoteOk) {
>>>> +                TestLibrary.bomb("Remote registry is up, an 
>>>> Exception is expected!");
>>>> +            }
>>>>          } catch (Throwable e) {
>>>>              if (remoteOk) {
>>>>                  System.err.println("EXPECTING PORT IN USE 
>>>> EXCEPTION:");
>>>>
>>>
>>
>