RFR: 8062744 jdk.net.Sockets.setOption/getOption does not support IP_TOS

[Chris Hegarty]

On 4 Nov 2014, at 16:24, Michael McMahon <michael.x.mcmahon at oracle.com> wrote:

> On 04/11/14 14:45, Michael McMahon wrote:
>> On 04/11/14 12:49, Chris Hegarty wrote:
>>> On 4 Nov 2014, at 12:05, Michael McMahon <michael.x.mcmahon at oracle.com> wrote:
>>> 
>>>> On 04/11/14 11:18, Chris Hegarty wrote:
>>>>> On 4 Nov 2014, at 11:15, Michael McMahon <michael.x.mcmahon at oracle.com> wrote:
>>>>> 
>>>>>> On 04/11/14 11:10, Alan Bateman wrote:
>>>>>>> On 04/11/2014 11:00, Michael McMahon wrote:
>>>>>>>> Thanks Alan. How about I just split the test and check the option setting behavior in OptionsTest.java
>>>>>>>> which doesn't have any reference to the jdk.net.Sockets APIs and then check the Sockets.supportedOptions()
>>>>>>>> method in the new test?
>>>>>>> That sounds okay too, I assume it's possible to come up with a good name for the new test too.
>>>>>>> 
>>>>>>> -Alan
>>>>>> SupportedOptions.java maybe.
>>>>> That would be fine, or
>>>>>   SupportedOptionsTest.java, or
>>>>>   StandardSocketOptionsTest.java
>>>>> 
>>>>> -Chris.
>>>> Ok. I've just realised that OptionsTest.java is testing all of the legacy option set/get calls
>>>> together with the new setOption(), getOption() calls. Problem with this:
>>>> 
>>>> 1) There is no legacy API for setting IP_TOS on ServerSocket
>>>> 
>>>> 2) The test is not in 8u-dev (because the setOption/getOption API is 9 only)
>>>> 
>>>> So, I'd like to stick with the original test (renamed).
>>> OK, I see the issue now.
>>> 
>>> Just another comment when I looked into this further; Should java/net/SocketImpl.java also add IP_TOS to it’s list of server socket options?
>> 
>> Right. Good catch. I've updated the webrev including the test to catch the above.
>> 
>> http://cr.openjdk.java.net/~michaelm/8062744/webrev.2/
> 
> here is the JDK 8 version. Almost the same, but only the changes relating to jdk.net.Sockets
> apply in this version.
> 
> http://cr.openjdk.java.net/~michaelm/8062744/webrev.1.jdk8/

Looks ok to me.

-Chris.

> 
> Thanks
> Michael