On Mon, 5 May 2025 10:52:12 GMT, Alan Bateman <al...@openjdk.org> wrote:

>> src/java.base/share/classes/sun/nio/ch/NioSocketImpl.java line 453:
>> 
>>> 451:     protected void create(boolean stream) throws IOException {
>>> 452:         if (!stream) {
>>> 453:             throw new IllegalArgumentException("datagram socket 
>>> creation not supported");
>> 
>> My initial thought was to just `assert` the `stream` value here. Then I 
>> noticed the `test/jdk/java/net/SocketImpl/BadUsages.java` test (updated as 
>> part of this PR) and thought that it might be better to do an actual check 
>> here and have it throw `IllegalArgumentException`, to allow for this 
>> behaviour to be verified.
>> 
>> I however don't have a strong opinion about this. So if `assert` is enough, 
>> then I'll switch this to an assert and then remove the updated test method 
>> in the `BasUsages.java`.
>
> This method can only throw IOException so I think it will need to throw 
> IOException. It should happen of course, at least not unless we have a bug in 
> the Socket code. Can you change the exception message to start with "Datagram 
> socket ..." so it's consistent with the other exception messages.

Done - I've updated the PR to follow these suggestions. The `BadUsages` test 
passes with this change.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/25031#discussion_r2073244098

Reply via email to