Re: 8213210: Change ServerSocket(SocketImpl impl) constructor to protected access

[Chris Hegarty]

> On 31 Oct 2018, at 19:26, Brian Burkhalter  
> wrote:
> 
> ...
> 
> --- a/src/java.base/share/classes/java/net/ServerSocket.java
> +++ b/src/java.base/share/classes/java/net/ServerSocket.java
> @@ -76,10 +76,16 @@
>  private boolean oldImpl = false;
>  
>  /**
> - * Package-private constructor to create a ServerSocket associated with
> - * the given SocketImpl.
> + * Creates a server socket with a user-specified {@code SocketImpl}.
> + *
> + * @param  impl an instance of a SocketImpl the subclass
> + * wishes to use on the ServerSocket.
> + *
> + * @throws NullPointerException if impl is {@code null}
> + *
> + * @since 12
>   */
> -ServerSocket(SocketImpl impl) {
> +protected ServerSocket(SocketImpl impl) {
>  this.impl = impl;
>  impl.setServerSocket(this);
>  }

I think this is good. I added myself as reviewer on the CSR too.

-Chris.

Re: 8213210: Change ServerSocket(SocketImpl impl) constructor to protected access

[Alan Bateman]

On 31/10/2018 19:26, Brian Burkhalter wrote:


On Oct 31, 2018, at 12:20 PM, Brian Burkhalter 
mailto:brian.burkhal...@oracle.com>> wrote:


-   ServerSocket(SocketImpl impl) {
+   protected ServerSocket(SocketImpl impl) {
      this.impl = impl;
      impl.setServerSocket(this); // <- NPE if impl == null
  }


Oops, it can throw null as indicated above. Revised patch below.


Good, I think looks okay.

-Alan

Re: 8213210: Change ServerSocket(SocketImpl impl) constructor to protected access

[Brian Burkhalter]

> On Oct 31, 2018, at 12:20 PM, Brian Burkhalter  
> wrote:
> 
> -ServerSocket(SocketImpl impl) {
> +protected ServerSocket(SocketImpl impl) {
>  this.impl = impl;
>  impl.setServerSocket(this); // <- NPE if impl == null
>  }

Oops, it can throw null as indicated above. Revised patch below.

Brian

--- a/src/java.base/share/classes/java/net/ServerSocket.java
+++ b/src/java.base/share/classes/java/net/ServerSocket.java
@@ -76,10 +76,16 @@
 private boolean oldImpl = false;
 
 /**
- * Package-private constructor to create a ServerSocket associated with
- * the given SocketImpl.
+ * Creates a server socket with a user-specified {@code SocketImpl}.
+ *
+ * @param  impl an instance of a SocketImpl the subclass
+ * wishes to use on the ServerSocket.
+ *
+ * @throws NullPointerException if impl is {@code null}
+ *
+ * @since 12
  */
-ServerSocket(SocketImpl impl) {
+protected ServerSocket(SocketImpl impl) {
 this.impl = impl;
 impl.setServerSocket(this);
 }

Re: 8213210: Change ServerSocket(SocketImpl impl) constructor to protected access

[Brian Burkhalter]

> On Oct 31, 2018, at 12:16 PM, Alan Bateman  wrote:
> 
> On 31/10/2018 19:13, Brian Burkhalter wrote:
>> https://bugs.openjdk.java.net/browse/JDK-8213210 
>> 
>> 
>> Please see diff included below. CSR to follow.
> One thing to check is whether ServerSocket specifies null handling anywhere. 
> I don't think it does so you might have to add an @throws NPE.

SocketImpl.serServerSocket() cannot throw NPE so I don’t see that the updated 
ctor can throw it either:

void setServerSocket(ServerSocket soc) {
this.serverSocket = soc;
}

-ServerSocket(SocketImpl impl) {
+protected ServerSocket(SocketImpl impl) {
 this.impl = impl;
 impl.setServerSocket(this);
 }

Brian

Re: 8213210: Change ServerSocket(SocketImpl impl) constructor to protected access

[Alan Bateman]

On 31/10/2018 19:13, Brian Burkhalter wrote:

https://bugs.openjdk.java.net/browse/JDK-8213210

Please see diff included below. CSR to follow.
One thing to check is whether ServerSocket specifies null handling 
anywhere. I don't think it does so you might have to add an @throws NPE.


-Alan

8213210: Change ServerSocket(SocketImpl impl) constructor to protected access

[Brian Burkhalter]

https://bugs.openjdk.java.net/browse/JDK-8213210

Please see diff included below. CSR to follow.

Thanks,

Brian

--- a/src/java.base/share/classes/java/net/ServerSocket.java
+++ b/src/java.base/share/classes/java/net/ServerSocket.java
@@ -76,10 +76,13 @@
 private boolean oldImpl = false;
 
 /**
- * Package-private constructor to create a ServerSocket associated with
- * the given SocketImpl.
+ * Creates a server socket with a user-specified {@code SocketImpl}.
+ *
+ * @param  impl an instance of a SocketImpl the subclass
+ * wishes to use on the ServerSocket.
+ * @since 12
  */
-ServerSocket(SocketImpl impl) {
+protected ServerSocket(SocketImpl impl) {
 this.impl = impl;
 impl.setServerSocket(this);
 }