[PATCH] for Review: Fix for fix for PR 13102

[Mohan Embar <gnustuff at thisiscool dot com>]

Hi Michael,

This patch is a follow-up for this:

http://gcc.gnu.org/ml/java-patches/2003-q4/msg00398.html

...which I'm sorry I only skimmed before you
checked in.

I ran into problems when running your patch with
my network test. I think the problem is due
to the use of s.impl in ServerSocket.implAccept()
given that impl hasn't been initialized yet.
However, you made Socket.getImpl() private.

What I did was made Socket.getImpl() package-private
instead of private, but this then caused an
overload problem with NIOSocket. Therefore, I
renamed this to getSocketImpl().

While I was in Socket.java, I made a new
method which calls getSocketImpl() but eats
the SocketException. This simplified a few methods
like getPort(), getLocalPort(), etc. I called
this new method getImpl() (not to be confused
with the old getImpl() which is now getSocketImpl() :) )

There was one final thing. In a lot of methods,
you had if (getImpl() != null) which is always
true. What I think you meant here is isConnected().
I changed this, but please review.

With this patch, my networking test now passes
with the (linux,mingw) cross compiler, but I
don't have a Linux native compiler to test with.
(I could build one if you want.)

Let me know how you want to proceed with this.

-- Mohan
http://www.thisiscool.com/
http://www.animalsong.org/

ChangeLog
2003-11-20  Mohan Embar  <gnustuff@thisiscool.com>

	* java/net/ServerSocket.java (implAccept): Use
	Socket.getSocketImpl() instead of Socket.impl
	* java/net/Socket.java
	Changed getImpl() to getSocketImpl(); changed from
	private to package-private.
	(impl): Made private.
	(various): Use getSocketImpl() instead of getImpl().
	(getInetAddress): Use getImpl() instead of getSocketImpl() +
	suppressing SocketException.
	(getPort): Likewise.
	(getLocalPort): Likewise.
	(getLocalSocketAddress): Likewise.
	(getRemoteSocketAddress): Likewise.
	(getLocalPort): Likewise.
	(toString): Likewise.
	(getInputStream): Use isConnected() instead testing impl for
	nullness.
	(getOutputStream): Likewise.
	(close): Likewise.
	(shutdownInput): Likewise.
	(shutdownOutput): Likewise.
	(getSocketImpl): Renamed from getImpl().
	(getImpl): Calls getSocketImpl() and eats any SocketException.

Index: java/net/ServerSocket.java
===================================================================
RCS file: /cvs/gcc/gcc/libjava/java/net/ServerSocket.java,v
retrieving revision 1.28
diff -u -2 -r1.28 ServerSocket.java
--- java/net/ServerSocket.java	11 Oct 2003 18:01:35 -0000	1.28
+++ java/net/ServerSocket.java	20 Nov 2003 17:27:55 -0000
@@ -330,5 +330,5 @@
       throw new IllegalBlockingModeException();
 	    
-    impl.accept(s.impl);
+    impl.accept(s.getSocketImpl());
   }
 
Index: java/net/Socket.java
===================================================================
RCS file: /cvs/gcc/gcc/libjava/java/net/Socket.java,v
retrieving revision 1.29
diff -u -2 -r1.29 Socket.java
--- java/net/Socket.java	19 Nov 2003 08:34:21 -0000	1.29
+++ java/net/Socket.java	20 Nov 2003 17:27:56 -0000
@@ -83,5 +83,5 @@
    * The implementation object to which calls are redirected
    */
-  SocketImpl impl;
+  private SocketImpl impl;
 
   private boolean implCreated = false;
@@ -299,23 +299,4 @@
   }
 
-  private SocketImpl getImpl()
-    throws SocketException
-  {
-    try
-      {
-	if (!implCreated)
-	  {
-	    impl.create(true);
-	    implCreated = true;
-	  }
-      }
-    catch (IOException e)
-      {
-	throw new SocketException(e.getMessage());
-      }
-
-    return impl;
-  }
-
   /**
    * Binds the socket to the givent local address/port
@@ -348,5 +329,5 @@
     try
       {
-        getImpl().bind (tmp.getAddress(), tmp.getPort());
+        getSocketImpl().bind (tmp.getAddress(), tmp.getPort());
       }
     catch (IOException exception)
@@ -418,5 +399,5 @@
     try
       {
-        getImpl().connect (endpoint, timeout);
+        getSocketImpl().connect (endpoint, timeout);
       }
     catch (IOException exception)
@@ -448,14 +429,5 @@
       return null;
 
-    try
-      {
-	return getImpl().getInetAddress();
-      }
-    catch (SocketException e)
-      {
-	// This cannot happen as we are connected.
-      }
-
-    return null;
+    return getSocketImplNoException().getInetAddress();
   }
 
@@ -474,5 +446,6 @@
     try
       {
-        addr = (InetAddress) getImpl().getOption(SocketOptions.SO_BINDADDR);
+        addr = (InetAddress) getSocketImpl().
+            getOption(SocketOptions.SO_BINDADDR);
       }
     catch(SocketException e)
@@ -507,15 +480,5 @@
       return 0;
 
-    try
-      {
-	if (getImpl() != null)
-	  return getImpl().getPort();
-      }
-    catch (SocketException e)
-      {
-	// This cannot happen as we are connected.
-      }
-
-    return -1;
+    return getSocketImplNoException().getPort();
   }
 
@@ -531,15 +494,5 @@
       return -1;
     
-    try
-      {
-	if (getImpl() != null)
-	  return getImpl().getLocalPort();
-      }
-    catch (SocketException e)
-      {
-	// This cannot happen as we are bound.
-      }
-
-    return -1;
+    return getSocketImplNoException().getLocalPort();
   }
 
@@ -556,14 +509,6 @@
     
     InetAddress addr = getLocalAddress ();
-
-    try
-      {
-	return new InetSocketAddress (addr, getImpl().getLocalPort());
-      }
-    catch (SocketException e)
-      {
-	// This cannot happen as we are bound.
-	return null;
-      }
+    return new InetSocketAddress (addr,
+    	getSocketImplNoException().getLocalPort());
   }
 
@@ -579,13 +524,7 @@
       return null;
 
-    try
-      {
-	return new InetSocketAddress (getImpl().getInetAddress (), getImpl().getPort ());
-      }
-    catch (SocketException e)
-      {
-	// This cannot happen as we are connected.
-	return null;
-      }
+    return new InetSocketAddress (
+	getSocketImplNoException().getInetAddress (),
+	getSocketImplNoException().getPort ());
   }
 
@@ -599,8 +538,9 @@
   public InputStream getInputStream () throws IOException
   {
-    if (getImpl() != null)
-      return getImpl().getInputStream();
+    if (!isConnected ())
+	throw new IOException("Not connected");
+	
+    return getSocketImpl().getInputStream();
 
-    throw new IOException("Not connected");
   }
 
@@ -614,8 +554,8 @@
   public OutputStream getOutputStream () throws IOException
   {
-    if (getImpl() != null)
-      return getImpl().getOutputStream();
-
-    throw new IOException("Not connected");
+    if (!isConnected ())
+	throw new IOException("Not connected");
+	
+    return getSocketImpl().getOutputStream();
   }
 
@@ -631,5 +571,5 @@
   public void setTcpNoDelay (boolean on)  throws SocketException
   {
-    getImpl().setOption(SocketOptions.TCP_NODELAY, new Boolean(on));
+    getSocketImpl().setOption(SocketOptions.TCP_NODELAY, new Boolean(on));
   }
 
@@ -648,5 +588,5 @@
   public boolean getTcpNoDelay() throws SocketException
   {
-    Object on = getImpl().getOption(SocketOptions.TCP_NODELAY);
+    Object on = getSocketImpl().getOption(SocketOptions.TCP_NODELAY);
   
     if (on instanceof Boolean)
@@ -683,9 +623,11 @@
           linger = 65535;
 
-        getImpl().setOption(SocketOptions.SO_LINGER, new Integer(linger));
+        getSocketImpl().setOption(SocketOptions.SO_LINGER,
+	  new Integer(linger));
       }
     else
       {
-        getImpl().setOption(SocketOptions.SO_LINGER, new Boolean(false));
+        getSocketImpl().setOption(SocketOptions.SO_LINGER,
+	  new Boolean(false));
       }
   }
@@ -709,5 +651,5 @@
   public int getSoLinger() throws SocketException
   {
-    Object linger = getImpl().getOption(SocketOptions.SO_LINGER);
+    Object linger = getSocketImpl().getOption(SocketOptions.SO_LINGER);
 
     if (linger instanceof Integer)
@@ -729,5 +671,5 @@
   public void sendUrgentData (int data) throws IOException
   {
-    getImpl().sendUrgentData (data);
+    getSocketImpl().sendUrgentData (data);
   }
 
@@ -743,5 +685,5 @@
   public void setOOBInline (boolean on) throws SocketException
   {
-    getImpl().setOption(SocketOptions.SO_OOBINLINE, new Boolean(on));
+    getSocketImpl().setOption(SocketOptions.SO_OOBINLINE, new Boolean(on));
   }
 
@@ -755,5 +697,5 @@
   public boolean getOOBInline () throws SocketException
   {
-    Object buf = getImpl().getOption(SocketOptions.SO_OOBINLINE);
+    Object buf = getSocketImpl().getOption(SocketOptions.SO_OOBINLINE);
 
     if (buf instanceof Boolean)
@@ -785,5 +727,5 @@
       throw new IllegalArgumentException("SO_TIMEOUT value must be >= 0");
       
-    getImpl().setOption(SocketOptions.SO_TIMEOUT, new Integer(timeout));
+    getSocketImpl().setOption(SocketOptions.SO_TIMEOUT, new Integer(timeout));
   }
 
@@ -807,5 +749,5 @@
   public synchronized int getSoTimeout () throws SocketException
   {
-    Object timeout = getImpl().getOption(SocketOptions.SO_TIMEOUT);
+    Object timeout = getSocketImpl().getOption(SocketOptions.SO_TIMEOUT);
     if (timeout instanceof Integer)
       return(((Integer)timeout).intValue());
@@ -831,5 +773,5 @@
       throw new IllegalArgumentException("SO_SNDBUF value must be > 0");
     
-    getImpl().setOption(SocketOptions.SO_SNDBUF, new Integer(size));
+    getSocketImpl().setOption(SocketOptions.SO_SNDBUF, new Integer(size));
   }
 
@@ -847,5 +789,5 @@
   public int getSendBufferSize () throws SocketException
   {
-    Object buf = getImpl().getOption(SocketOptions.SO_SNDBUF);
+    Object buf = getSocketImpl().getOption(SocketOptions.SO_SNDBUF);
 
     if (buf instanceof Integer)
@@ -872,5 +814,5 @@
       throw new IllegalArgumentException("SO_RCVBUF value must be > 0");
       
-    getImpl().setOption(SocketOptions.SO_RCVBUF, new Integer(size));
+    getSocketImpl().setOption(SocketOptions.SO_RCVBUF, new Integer(size));
   }
 
@@ -888,5 +830,5 @@
   public int getReceiveBufferSize () throws SocketException
   {
-    Object buf = getImpl().getOption(SocketOptions.SO_RCVBUF);
+    Object buf = getSocketImpl().getOption(SocketOptions.SO_RCVBUF);
 
     if (buf instanceof Integer)
@@ -908,5 +850,5 @@
   public void setKeepAlive (boolean on) throws SocketException
   {
-    getImpl().setOption(SocketOptions.SO_KEEPALIVE, new Boolean(on));
+    getSocketImpl().setOption(SocketOptions.SO_KEEPALIVE, new Boolean(on));
   }
 
@@ -923,5 +865,5 @@
   public boolean getKeepAlive () throws SocketException
   {
-    Object buf = getImpl().getOption(SocketOptions.SO_KEEPALIVE);
+    Object buf = getSocketImpl().getOption(SocketOptions.SO_KEEPALIVE);
 
     if (buf instanceof Boolean)
@@ -938,6 +880,6 @@
   public synchronized void close ()  throws IOException
   {
-    if (getImpl() != null)
-      getImpl().close();
+    if (isConnected())
+      getSocketImpl().close();
 
     if (getChannel() != null)
@@ -954,17 +896,10 @@
   public String toString ()
   {
-    try
-      {
-	if (isConnected())
-	  return ("Socket[addr=" + getImpl().getInetAddress()
-		  + ",port=" + getImpl().getPort()
-		  + ",localport=" + getImpl().getLocalPort());
-      }
-    catch (SocketException e)
-      {
-	// This cannot happen as we are connected.
-      }
-
-    return "Socket[unconnected]";
+    if (isConnected())
+	return ("Socket[addr=" + getSocketImplNoException().getInetAddress()
+	  + ",port=" + getSocketImplNoException().getPort()
+	  + ",localport=" + getSocketImplNoException().getLocalPort());
+    else
+	return "Socket[unconnected]";
   }
 
@@ -1011,6 +946,6 @@
   public void shutdownInput() throws IOException
   {
-    if (getImpl() != null)
-      getImpl().shutdownInput();
+    if (isConnected())
+      getSocketImpl().shutdownInput();
 
     inputShutdown = true;
@@ -1026,6 +961,6 @@
   public void shutdownOutput() throws IOException
   {
-    if (getImpl() != null)
-      getImpl().shutdownOutput();
+    if (isConnected())
+      getSocketImpl().shutdownOutput();
     
     outputShutdown = true;
@@ -1053,5 +988,5 @@
   public boolean getReuseAddress () throws SocketException
   {
-    Object reuseaddr = getImpl().getOption (SocketOptions.SO_REUSEADDR);
+    Object reuseaddr = getSocketImpl().getOption (SocketOptions.SO_REUSEADDR);
 
     if (!(reuseaddr instanceof Boolean))
@@ -1070,5 +1005,5 @@
   public void setReuseAddress (boolean on) throws SocketException
   {
-    getImpl().setOption (SocketOptions.SO_REUSEADDR, new Boolean (on));
+    getSocketImpl().setOption (SocketOptions.SO_REUSEADDR, new Boolean (on));
   }
 
@@ -1084,5 +1019,5 @@
   public int getTrafficClass () throws SocketException
   {
-    Object obj = getImpl().getOption(SocketOptions.IP_TOS);
+    Object obj = getSocketImpl().getOption(SocketOptions.IP_TOS);
 
     if (obj instanceof Integer)
@@ -1109,5 +1044,5 @@
       throw new IllegalArgumentException();
 
-    getImpl().setOption (SocketOptions.IP_TOS, new Integer (tc));
+    getSocketImpl().setOption (SocketOptions.IP_TOS, new Integer (tc));
   }
 
@@ -1121,5 +1056,5 @@
     try
       {
-	return getImpl().getInetAddress () != null;
+	return getSocketImpl().getInetAddress () != null;
       }
     catch (SocketException e)
@@ -1167,4 +1102,35 @@
   {
     return outputShutdown;
+  }
+
+  SocketImpl getSocketImpl()
+    throws SocketException
+  {
+    try
+      {
+	if (!implCreated)
+	  {
+	    impl.create(true);
+	    implCreated = true;
+	  }
+      }
+    catch (IOException e)
+      {
+	throw new SocketException(e.getMessage());
+      }
+
+    return impl;
+  }
+
+  private SocketImpl getSocketImplNoException()
+  {
+    try
+      {
+	return getSocketImpl();
+      }
+    catch (SocketException e)
+      {
+	return null;
+      }
   }
 }