RFR[8233307]: MulticastSocket getOption(IP_MULTICAST_IF) returns interface when not set

[Daniel Fuchs]

Looks good to me Patrick!

Thanks for the test updates :-)

best regards,

-- daniel

On 28/11/2019 13:01, Patrick Concannon wrote:
> Hi Chris,
> 
> Thanks for your feedback.
> 
> I've incorporated your comments into the fix, and the changes can be 
> found in the updated webrev below.
> 
> webrev: http://cr.openjdk.java.net/~pconcannon/8233307/webrevs/webrev.02/
> 
> Kind regards,
> 
> Patrick
> 
> 
> On 28/11/2019 09:36, Chris Hegarty wrote:
>> Patrick,
>>
>>> On 27 Nov 2019, at 14:54, Patrick Concannon 
>>> <patrick.concannon at oracle.com> wrote:
>>>
>>> Hi,
>>>
>>>
>>> Could someone please review my fix and CSR for issue JDK-8233307 
>>> 'MulticastSocket getOption(IP_MULTICAST_IF) returns interface when 
>>> not set'?
>>>
>>> The IP_MULTICAST_IF socket option is specified to have an initial 
>>> value of null to mean that the interface has not been set. However, 
>>> if MulticastSocket.getOption(IP_MULTICAST_IF) is called when the 
>>> interface is not set, it returns a dummy NetworkInterface.
>>>
>>> This fix changes the getOption() method to return 'null' as specified 
>>> instead of a dummy networkInterface.
>>>
>>>
>>> bug: https://bugs.openjdk.java.net/browse/JDK-8233307
>>>
>>> csr: https://bugs.openjdk.java.net/browse/JDK-8234811
>>> webrev: 
>>> http://cr.openjdk.java.net/~pconcannon/8233307/webrevs/webrev.01/
>> This looks good. Some small comments on the test:
>>
>> 1) The test should use the (test library) NetworkConfiguration
>>     multicastInterfaces method, instead of retrieving all the system
>>     interfaces. The former will filter out potentially problematic system
>>     interfaces that could affect the stability of the test.
>>
>> 2) Enumeration :-( Suggest [ subject to the above comment ]
>>     a) NetworkInterface.networkInterfaces().collect(toList()) , or
>>     b) Collections.list(NetworkInterface.getNetworkInterfaces())
>>
>> 3) While the test does run in othervm mode, it should probably be a good
>>     citizen and close the sockets that it opens.
>>
>> -Chris
>>