public inbox for java-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* RFC: Untested Win32 InetAddress tweak
@ 2006-09-04 14:15 Gary Benson
  2006-09-06 16:58 ` Tom Tromey
  2006-10-01 17:36 ` Marco Trudel
  0 siblings, 2 replies; 15+ messages in thread
From: Gary Benson @ 2006-09-04 14:15 UTC (permalink / raw)
  To: java-patches

[-- Attachment #1: Type: text/plain, Size: 434 bytes --]

Hi all,

The last part of java.net.InetAddress.lookup (a native method) is the
same in both Posix and Win32 variants except for a security check that
was removed from the Posix variant in August 2004.  The attached patch
removes this check from the Win32, synchronising the two.

I don't have the ability to test this, so could someone try it for me?
Alternatively, is this a trivial enough change to just commit blind?

Cheers,
Gary

[-- Attachment #2: win32-no-security-check.patch --]
[-- Type: text/plain, Size: 1779 bytes --]

Index: ChangeLog
===================================================================
--- ChangeLog	(revision 116678)
+++ ChangeLog	(working copy)
@@ -1,3 +1,9 @@
+2006-09-04  Gary Benson  <gbenson@redhat.com>
+
+	* java/net/natInetAddressWin32.cc (lookup): Remove security
+	check.
+	* java/net/InetAddress.java (checkConnect): Removed.
+
 2006-09-01  Geoffrey Keating  <geoffk@apple.com>
 
 	* testsuite/libjava.jni/jni.exp (gcj_jni_invocation_test_one):
Index: java/net/natInetAddressWin32.cc
===================================================================
--- java/net/natInetAddressWin32.cc	(revision 116678)
+++ java/net/natInetAddressWin32.cc	(working copy)
@@ -94,13 +94,6 @@
     {
       if (!all)
         host = JvNewStringUTF (hptr->h_name);
-      java::lang::SecurityException *ex = checkConnect (host);
-      if (ex != NULL)
-        {
-          if (iaddr == NULL || iaddr->addr == NULL)
-            throw ex;
-          hptr = NULL;
-        }
     }
   if (hptr == NULL)
     {
Index: java/net/InetAddress.java
===================================================================
--- java/net/InetAddress.java	(revision 116678)
+++ java/net/InetAddress.java	(working copy)
@@ -352,25 +352,6 @@
     return new InetAddress [count];
   }
 
-  /* Helper function due to a CNI limitation.  */
-  private static SecurityException checkConnect (String hostname)
-  {
-    SecurityManager s = System.getSecurityManager();
-    
-    if (s == null)
-      return null;
-    
-    try
-      {
-	s.checkConnect (hostname, -1);
-	return null;
-      }
-    catch (SecurityException ex)
-      {
-	return ex;
-      }
-  }
-
   /**
    * Returns the IP address of this object as a String.  The address is in
    * the dotted octet notation, for example, "127.0.0.1".

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: RFC: Untested Win32 InetAddress tweak
  2006-09-04 14:15 RFC: Untested Win32 InetAddress tweak Gary Benson
@ 2006-09-06 16:58 ` Tom Tromey
  2006-09-07  7:36   ` Gary Benson
  2006-10-01 17:36 ` Marco Trudel
  1 sibling, 1 reply; 15+ messages in thread
From: Tom Tromey @ 2006-09-06 16:58 UTC (permalink / raw)
  To: Gary Benson; +Cc: java-patches

>>>>> "Gary" == Gary Benson <gbenson@redhat.com> writes:

Gary> I don't have the ability to test this, so could someone try it for me?
Gary> Alternatively, is this a trivial enough change to just commit blind?

Please check it in.

Gary> -  /* Helper function due to a CNI limitation.  */

Wow, this must be truly ancient code.
We haven't had this CNI limitation for years and years.

Tom

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: RFC: Untested Win32 InetAddress tweak
  2006-09-06 16:58 ` Tom Tromey
@ 2006-09-07  7:36   ` Gary Benson
  0 siblings, 0 replies; 15+ messages in thread
From: Gary Benson @ 2006-09-07  7:36 UTC (permalink / raw)
  To: java-patches

Tom Tromey wrote:
> >>>>> "Gary" == Gary Benson <gbenson@redhat.com> writes:
> 
> Gary> I don't have the ability to test this, so could someone try
> Gary> it for me?  Alternatively, is this a trivial enough change
> Gary> to just commit blind?
> 
> Please check it in.
> 
> Gary> -  /* Helper function due to a CNI limitation.  */
> 
> Wow, this must be truly ancient code.  We haven't had this CNI
> limitation for years and years.

I meant to ask someone about that.  I'll take that out too...

Cheers,
Gary

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: RFC: Untested Win32 InetAddress tweak
  2006-09-04 14:15 RFC: Untested Win32 InetAddress tweak Gary Benson
  2006-09-06 16:58 ` Tom Tromey
@ 2006-10-01 17:36 ` Marco Trudel
  2006-10-01 18:00   ` Mohan Embar
  1 sibling, 1 reply; 15+ messages in thread
From: Marco Trudel @ 2006-10-01 17:36 UTC (permalink / raw)
  To: java-patches

Hello Gary

Gary Benson wrote:
> Hi all,
> 
> The last part of java.net.InetAddress.lookup (a native method) is the
> same in both Posix and Win32 variants except for a security check that
> was removed from the Posix variant in August 2004.  The attached patch
> removes this check from the Win32, synchronising the two.
> 
> I don't have the ability to test this, so could someone try it for me?
> Alternatively, is this a trivial enough change to just commit blind?

Something must have went wrong. java/net/InetAddress.java was changed 
according this patch, java/net/natInetAddressWin32.cc has not. This 
leads to an error on compilation. Please commit the rest as well...

Here's the link to the trunk head, you can see that the code is still there:
http://gcc.gnu.org/viewcvs/trunk/libjava/java/net/natInetAddressWin32.cc?view=markup


Marco


> Cheers,
> Gary
> 
> 
> ------------------------------------------------------------------------
> 
> Index: ChangeLog
> ===================================================================
> --- ChangeLog	(revision 116678)
> +++ ChangeLog	(working copy)
> @@ -1,3 +1,9 @@
> +2006-09-04  Gary Benson  <gbenson@redhat.com>
> +
> +	* java/net/natInetAddressWin32.cc (lookup): Remove security
> +	check.
> +	* java/net/InetAddress.java (checkConnect): Removed.
> +
>  2006-09-01  Geoffrey Keating  <geoffk@apple.com>
>  
>  	* testsuite/libjava.jni/jni.exp (gcj_jni_invocation_test_one):
> Index: java/net/natInetAddressWin32.cc
> ===================================================================
> --- java/net/natInetAddressWin32.cc	(revision 116678)
> +++ java/net/natInetAddressWin32.cc	(working copy)
> @@ -94,13 +94,6 @@
>      {
>        if (!all)
>          host = JvNewStringUTF (hptr->h_name);
> -      java::lang::SecurityException *ex = checkConnect (host);
> -      if (ex != NULL)
> -        {
> -          if (iaddr == NULL || iaddr->addr == NULL)
> -            throw ex;
> -          hptr = NULL;
> -        }
>      }
>    if (hptr == NULL)
>      {
> Index: java/net/InetAddress.java
> ===================================================================
> --- java/net/InetAddress.java	(revision 116678)
> +++ java/net/InetAddress.java	(working copy)
> @@ -352,25 +352,6 @@
>      return new InetAddress [count];
>    }
>  
> -  /* Helper function due to a CNI limitation.  */
> -  private static SecurityException checkConnect (String hostname)
> -  {
> -    SecurityManager s = System.getSecurityManager();
> -    
> -    if (s == null)
> -      return null;
> -    
> -    try
> -      {
> -	s.checkConnect (hostname, -1);
> -	return null;
> -      }
> -    catch (SecurityException ex)
> -      {
> -	return ex;
> -      }
> -  }
> -
>    /**
>     * Returns the IP address of this object as a String.  The address is in
>     * the dotted octet notation, for example, "127.0.0.1".

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: RFC: Untested Win32 InetAddress tweak
  2006-10-01 17:36 ` Marco Trudel
@ 2006-10-01 18:00   ` Mohan Embar
  2006-10-02  7:19     ` Marco Trudel
  0 siblings, 1 reply; 15+ messages in thread
From: Mohan Embar @ 2006-10-01 18:00 UTC (permalink / raw)
  To: Marco Trudel; +Cc: Gary Benson, java-patches

Hi All,

>> The last part of java.net.InetAddress.lookup (a native method) is the
>> same in both Posix and Win32 variants except for a security check that
>> was removed from the Posix variant in August 2004.  The attached patch
>> removes this check from the Win32, synchronising the two.
>> 
>> I don't have the ability to test this, so could someone try it for me?
>> Alternatively, is this a trivial enough change to just commit blind?
>
>Something must have went wrong. java/net/InetAddress.java was changed 
>according this patch, java/net/natInetAddressWin32.cc has not. This 
>leads to an error on compilation. Please commit the rest as well...
>
>Here's the link to the trunk head, you can see that the code is still there:
>http://gcc.gnu.org/viewcvs/trunk/libjava/java/net/natInetAddressWin32.cc?view=markup

I agree with Marco. Although I understand that Gary probably can't
commit the rest without approval, I can also confirm that current SVN HEAD
breaks for Win32 without Gary's patch + my minor address-of corrections.

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




^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: RFC: Untested Win32 InetAddress tweak
  2006-10-01 18:00   ` Mohan Embar
@ 2006-10-02  7:19     ` Marco Trudel
  2006-10-02 10:55       ` FYI: Win32 InetAddress fix Gary Benson
  0 siblings, 1 reply; 15+ messages in thread
From: Marco Trudel @ 2006-10-02  7:19 UTC (permalink / raw)
  To: gnustuff; +Cc: Gary Benson, java-patches

Mohan Embar wrote:
> Hi All,
> 
>>> The last part of java.net.InetAddress.lookup (a native method) is the
>>> same in both Posix and Win32 variants except for a security check that
>>> was removed from the Posix variant in August 2004.  The attached patch
>>> removes this check from the Win32, synchronising the two.
>>>
>>> I don't have the ability to test this, so could someone try it for me?
>>> Alternatively, is this a trivial enough change to just commit blind?
>> Something must have went wrong. java/net/InetAddress.java was changed 
>> according this patch, java/net/natInetAddressWin32.cc has not. This 
>> leads to an error on compilation. Please commit the rest as well...
>>
>> Here's the link to the trunk head, you can see that the code is still there:
>> http://gcc.gnu.org/viewcvs/trunk/libjava/java/net/natInetAddressWin32.cc?view=markup
> 
> I agree with Marco. Although I understand that Gary probably can't
> commit the rest without approval,

There seems to be some misunderstanding. I'm talking about this patch: 
http://gcc.gnu.org/ml/java-patches/2006-q3/txt00034.txt
Tom gave his ok to commit that, but it seems only parts of it made it 
into the trunk. So I assume it would be ok to commit the rest as well...
But actually that wouldn't help much. With the rest of that patch, GCJ 
would compile again for win32, but InetAddress throws an Exception at 
runtime:

SocketAddress sock = new SocketAddress(12345); throws a 
NullPointerException in natInetAddressWin32.cc at
    JvSynchronize sync (java::net::InetAddress::loopbackAddress);

I think this has been fixed in the latest patches to this topics. But 
these are not committed and break compilation again.


 > I can also confirm that current SVN HEAD breaks for
 > Win32 without Gary's patch + my minor address-of corrections.

I think you're talking about the second last patch 
(http://gcc.gnu.org/ml/java-patches/2006-q3/msg00448.html) that hasn't 
been committed yet. Gary updated that after your feedback:
http://gcc.gnu.org/ml/java-patches/2006-q3/msg00486.html

Gary, does this latest patch include all changes that are not yet in 
trunk? If not, can you please post an updated patch that includes 
everything even the missing parts of old patches? I'd like to test the 
win32 part.
If yes, it breaks at least the compilation of a linux to windows 
cross-GCJ starting with "Class InetAddress not found" and develops into 
java.lang.Object not found.


Can we either roll the complete InetAddress tweak back or get the rest 
fixed and committed as well?


Marco

^ permalink raw reply	[flat|nested] 15+ messages in thread

* FYI: Win32 InetAddress fix
  2006-10-02  7:19     ` Marco Trudel
@ 2006-10-02 10:55       ` Gary Benson
  2006-10-02 11:22         ` Marco Trudel
  2006-10-03 10:35         ` FYI: Win32 InetAddress fix (really) Gary Benson
  0 siblings, 2 replies; 15+ messages in thread
From: Gary Benson @ 2006-10-02 10:55 UTC (permalink / raw)
  To: java-patches

[-- Attachment #1: Type: text/plain, Size: 2389 bytes --]

Marco Trudel wrote:
> Mohan Embar wrote:
> > > > The last part of java.net.InetAddress.lookup (a native method)
> > > > is the same in both Posix and Win32 variants except for a
> > > > security check that was removed from the Posix variant in
> > > > August 2004.  The attached patch removes this check from the
> > > > Win32, synchronising the two.
> > > > 
> > > > I don't have the ability to test this, so could someone try
> > > > it for me?  Alternatively, is this a trivial enough change to
> > > > just commit blind?
> > > 
> > > Something must have went wrong. java/net/InetAddress.java was
> > > changed according this patch, java/net/natInetAddressWin32.cc
> > > has not. This leads to an error on compilation. Please commit
> > > the rest as well...
> > 
> > I agree with Marco. Although I understand that Gary probably can't
> > commit the rest without approval,
> 
> There seems to be some misunderstanding. I'm talking about this
> patch: http://gcc.gnu.org/ml/java-patches/2006-q3/txt00034.txt Tom
> gave his ok to commit that, but it seems only parts of it made it
> into the trunk. So I assume it would be ok to commit the rest as
> well...  But actually that wouldn't help much. With the rest of that
> patch, GCJ would compile again for win32, but InetAddress throws an
> Exception at runtime:
> 
> SocketAddress sock = new SocketAddress(12345); throws a 
> NullPointerException in natInetAddressWin32.cc at
>    JvSynchronize sync (java::net::InetAddress::loopbackAddress);
> 
> I think this has been fixed in the latest patches to this topics.
> But these are not committed and break compilation again.

This commit should fix the build failure and the NullPointerException.

FWIW what happened with all the different patches was that I wrote
http://gcc.gnu.org/ml/java-patches/2006-q3/msg00386.html thinking it
would be a stepping stone to a solution, but inbetween me posting it
to the list and Tom saying to check it in I realised that all that
code would be trashed anyway so I didn't bother committing it.

As far as the other patches go the only outstanding one is
http://gcc.gnu.org/ml/java-patches/2006-q3/msg00486.html (though I
need to remake it because of this commit).  That can't be committed
before the branch because the Posix part has a lot of conditional
stuff which might be broken.  I'll be committing that just as soon
as we branch.

Cheers,
Gary

[-- Attachment #2: inetaddress-win32-fix.patch --]
[-- Type: text/plain, Size: 1624 bytes --]

Index: ChangeLog
===================================================================
--- ChangeLog	(revision 117367)
+++ ChangeLog	(working copy)
@@ -1,3 +1,9 @@
+2006-10-02  Gary Benson  <gbenson@redhat.com>
+
+	* java/net/InetAddress.java
+	(checkConnect): Reinstated.
+	(loopbackAddress): Ensure initialized from non-null object.
+
 2006-09-30  Keith Seitz  <keiths@redhat.com>
 
 	* include/java-interp.h (prepared): Change type to pc_t.
Index: java/net/InetAddress.java
===================================================================
--- java/net/InetAddress.java	(revision 117367)
+++ java/net/InetAddress.java	(working copy)
@@ -101,6 +101,8 @@
     try
       {
 	LOCALHOST = getByAddress("localhost", new byte[] {127, 0, 0, 1});
+	// Some soon-to-be-removed native code synchronizes on this.
+	loopbackAddress = LOCALHOST;
       }
     catch (UnknownHostException e)
       {
@@ -793,7 +795,7 @@
   static native String getLocalHostname();
 
   // Some soon-to-be-removed native code synchronizes on this.
-  static InetAddress loopbackAddress = LOCALHOST;
+  static InetAddress loopbackAddress;
   
   // Some soon-to-be-removed code uses this old and broken method.
   InetAddress(byte[] ipaddr, String hostname)
@@ -805,9 +807,13 @@
       family = getFamily(ipaddr);
   }
 
-  // Some soon-to-be-removed native code uses this old method.
+  // Some soon-to-be-removed native code uses these old methods.
   private static InetAddress[] allocArray (int count)
   {
     return new InetAddress [count];
   }  
+  private static SecurityException checkConnect (String hostname)
+  {
+    return null;
+  }
 }

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: FYI: Win32 InetAddress fix
  2006-10-02 10:55       ` FYI: Win32 InetAddress fix Gary Benson
@ 2006-10-02 11:22         ` Marco Trudel
  2006-10-02 17:43           ` Tom Tromey
  2006-10-03 10:35         ` FYI: Win32 InetAddress fix (really) Gary Benson
  1 sibling, 1 reply; 15+ messages in thread
From: Marco Trudel @ 2006-10-02 11:22 UTC (permalink / raw)
  To: java-patches

Gary Benson wrote:
> Marco Trudel wrote:
>
> <bigsnip>
> 
> This commit should fix the build failure and the NullPointerException.
> 
> FWIW what happened with all the different patches was that I wrote
> http://gcc.gnu.org/ml/java-patches/2006-q3/msg00386.html thinking it
> would be a stepping stone to a solution, but inbetween me posting it
> to the list and Tom saying to check it in I realised that all that
> code would be trashed anyway so I didn't bother committing it.
> 
> As far as the other patches go the only outstanding one is
> http://gcc.gnu.org/ml/java-patches/2006-q3/msg00486.html (though I
> need to remake it because of this commit).  That can't be committed
> before the branch because the Posix part has a lot of conditional
> stuff which might be broken.  I'll be committing that just as soon
> as we branch.

When will we branch? Few days? Weeks? Fix date?
Are we talking about the 4.2 release?

Marco

> Cheers,
> Gary
> 
> 
> ------------------------------------------------------------------------
> 
> Index: ChangeLog
> ===================================================================
> --- ChangeLog	(revision 117367)
> +++ ChangeLog	(working copy)
> @@ -1,3 +1,9 @@
> +2006-10-02  Gary Benson  <gbenson@redhat.com>
> +
> +	* java/net/InetAddress.java
> +	(checkConnect): Reinstated.
> +	(loopbackAddress): Ensure initialized from non-null object.
> +
>  2006-09-30  Keith Seitz  <keiths@redhat.com>
>  
>  	* include/java-interp.h (prepared): Change type to pc_t.
> Index: java/net/InetAddress.java
> ===================================================================
> --- java/net/InetAddress.java	(revision 117367)
> +++ java/net/InetAddress.java	(working copy)
> @@ -101,6 +101,8 @@
>      try
>        {
>  	LOCALHOST = getByAddress("localhost", new byte[] {127, 0, 0, 1});
> +	// Some soon-to-be-removed native code synchronizes on this.
> +	loopbackAddress = LOCALHOST;
>        }
>      catch (UnknownHostException e)
>        {
> @@ -793,7 +795,7 @@
>    static native String getLocalHostname();
>  
>    // Some soon-to-be-removed native code synchronizes on this.
> -  static InetAddress loopbackAddress = LOCALHOST;
> +  static InetAddress loopbackAddress;
>    
>    // Some soon-to-be-removed code uses this old and broken method.
>    InetAddress(byte[] ipaddr, String hostname)
> @@ -805,9 +807,13 @@
>        family = getFamily(ipaddr);
>    }
>  
> -  // Some soon-to-be-removed native code uses this old method.
> +  // Some soon-to-be-removed native code uses these old methods.
>    private static InetAddress[] allocArray (int count)
>    {
>      return new InetAddress [count];
>    }  
> +  private static SecurityException checkConnect (String hostname)
> +  {
> +    return null;
> +  }
>  }

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: FYI: Win32 InetAddress fix
  2006-10-02 11:22         ` Marco Trudel
@ 2006-10-02 17:43           ` Tom Tromey
  0 siblings, 0 replies; 15+ messages in thread
From: Tom Tromey @ 2006-10-02 17:43 UTC (permalink / raw)
  To: Marco Trudel; +Cc: java-patches

>>>>> "Marco" == Marco Trudel <mtrudel@gmx.ch> writes:

Marco> When will we branch? Few days? Weeks? Fix date?

Nobody knows for sure.  It has looked a few weeks off for quite a
while now.

Marco> Are we talking about the 4.2 release?

Yes.

Tom

^ permalink raw reply	[flat|nested] 15+ messages in thread

* FYI: Win32 InetAddress fix (really)
  2006-10-02 10:55       ` FYI: Win32 InetAddress fix Gary Benson
  2006-10-02 11:22         ` Marco Trudel
@ 2006-10-03 10:35         ` Gary Benson
  2006-10-03 15:10           ` David Daney
  2006-10-03 15:30           ` Marco Trudel
  1 sibling, 2 replies; 15+ messages in thread
From: Gary Benson @ 2006-10-03 10:35 UTC (permalink / raw)
  To: java-patches

[-- Attachment #1: Type: text/plain, Size: 121 bytes --]

Hi all,

Marco pointed out that my last commit didn't fix the
NullPointerException.  This commit ought to.

Cheers,
Gary

[-- Attachment #2: inetaddress-merge-win32-really-fix.patch --]
[-- Type: text/plain, Size: 1874 bytes --]

Index: ChangeLog
===================================================================
--- ChangeLog	(revision 117370)
+++ ChangeLog	(working copy)
@@ -1,3 +1,8 @@
+2006-10-03  Gary Benson  <gbenson@redhat.com>
+
+	* java/net/InetAddress.java
+	(<clinit>): Reorder the static initializers.
+
 2006-10-02  Gary Benson  <gbenson@redhat.com>
 
 	* java/net/InetAddress.java
Index: java/net/InetAddress.java
===================================================================
--- java/net/InetAddress.java	(revision 117370)
+++ java/net/InetAddress.java	(working copy)
@@ -66,6 +66,24 @@
   private static final long serialVersionUID = 3286316764910316507L;
 
   /**
+   * Stores static localhost address object.
+   */
+  static InetAddress LOCALHOST;
+  static
+  {
+    try
+      {
+	LOCALHOST = getByAddress("localhost", new byte[] {127, 0, 0, 1});
+	// Some soon-to-be-removed native code synchronizes on this.
+	loopbackAddress = LOCALHOST;
+      }
+    catch (UnknownHostException e)
+      {
+	throw new RuntimeException("should never happen", e);
+      }
+  }    
+
+  /**
    * Dummy InetAddress, used to bind socket to any (all) network interfaces.
    */
   static InetAddress ANY_IF;
@@ -93,24 +111,6 @@
   }
   
   /**
-   * Stores static localhost address object.
-   */
-  static InetAddress LOCALHOST;
-  static
-  {
-    try
-      {
-	LOCALHOST = getByAddress("localhost", new byte[] {127, 0, 0, 1});
-	// Some soon-to-be-removed native code synchronizes on this.
-	loopbackAddress = LOCALHOST;
-      }
-    catch (UnknownHostException e)
-      {
-	throw new RuntimeException("should never happen", e);
-      }
-  }    
-
-  /**
    * The Serialized Form specifies that an int 'address' is saved/restored.
    * This class uses a byte array internally so we'll just do the conversion
    * at serialization time and leave the rest of the algorithm as is.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: FYI: Win32 InetAddress fix (really)
  2006-10-03 10:35         ` FYI: Win32 InetAddress fix (really) Gary Benson
@ 2006-10-03 15:10           ` David Daney
  2006-10-03 16:55             ` Andrew Haley
  2006-10-03 15:30           ` Marco Trudel
  1 sibling, 1 reply; 15+ messages in thread
From: David Daney @ 2006-10-03 15:10 UTC (permalink / raw)
  To: java-patches; +Cc: gbenson

Gary Benson wrote:
> +	// Some soon-to-be-removed native code synchronizes on this.
> +	loopbackAddress = LOCALHOST;
> +      }
> +    catch (UnknownHostException e)
> +      {
> +	throw new RuntimeException("should never happen", e);
> +      }
>   
Would it be better to throw an InternalError here rather than 
RuntimeException?  If it truly should never happen then it would be an 
InternalError if it did.  If it can happen, then it should be of some 
type the describes the problem.

David Daney



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: FYI: Win32 InetAddress fix (really)
  2006-10-03 10:35         ` FYI: Win32 InetAddress fix (really) Gary Benson
  2006-10-03 15:10           ` David Daney
@ 2006-10-03 15:30           ` Marco Trudel
  1 sibling, 0 replies; 15+ messages in thread
From: Marco Trudel @ 2006-10-03 15:30 UTC (permalink / raw)
  To: java-patches

Yep, this time it works :-)

thanks
Marco


Gary Benson wrote:
> Hi all,
> 
> Marco pointed out that my last commit didn't fix the
> NullPointerException.  This commit ought to.
> 
> Cheers,
> Gary
> 
> 
> ------------------------------------------------------------------------
> 
> Index: ChangeLog
> ===================================================================
> --- ChangeLog	(revision 117370)
> +++ ChangeLog	(working copy)
> @@ -1,3 +1,8 @@
> +2006-10-03  Gary Benson  <gbenson@redhat.com>
> +
> +	* java/net/InetAddress.java
> +	(<clinit>): Reorder the static initializers.
> +
>  2006-10-02  Gary Benson  <gbenson@redhat.com>
>  
>  	* java/net/InetAddress.java
> Index: java/net/InetAddress.java
> ===================================================================
> --- java/net/InetAddress.java	(revision 117370)
> +++ java/net/InetAddress.java	(working copy)
> @@ -66,6 +66,24 @@
>    private static final long serialVersionUID = 3286316764910316507L;
>  
>    /**
> +   * Stores static localhost address object.
> +   */
> +  static InetAddress LOCALHOST;
> +  static
> +  {
> +    try
> +      {
> +	LOCALHOST = getByAddress("localhost", new byte[] {127, 0, 0, 1});
> +	// Some soon-to-be-removed native code synchronizes on this.
> +	loopbackAddress = LOCALHOST;
> +      }
> +    catch (UnknownHostException e)
> +      {
> +	throw new RuntimeException("should never happen", e);
> +      }
> +  }    
> +
> +  /**
>     * Dummy InetAddress, used to bind socket to any (all) network interfaces.
>     */
>    static InetAddress ANY_IF;
> @@ -93,24 +111,6 @@
>    }
>    
>    /**
> -   * Stores static localhost address object.
> -   */
> -  static InetAddress LOCALHOST;
> -  static
> -  {
> -    try
> -      {
> -	LOCALHOST = getByAddress("localhost", new byte[] {127, 0, 0, 1});
> -	// Some soon-to-be-removed native code synchronizes on this.
> -	loopbackAddress = LOCALHOST;
> -      }
> -    catch (UnknownHostException e)
> -      {
> -	throw new RuntimeException("should never happen", e);
> -      }
> -  }    
> -
> -  /**
>     * The Serialized Form specifies that an int 'address' is saved/restored.
>     * This class uses a byte array internally so we'll just do the conversion
>     * at serialization time and leave the rest of the algorithm as is.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: FYI: Win32 InetAddress fix (really)
  2006-10-03 15:10           ` David Daney
@ 2006-10-03 16:55             ` Andrew Haley
  2006-10-03 17:23               ` David Daney
  0 siblings, 1 reply; 15+ messages in thread
From: Andrew Haley @ 2006-10-03 16:55 UTC (permalink / raw)
  To: David Daney; +Cc: java-patches, gbenson

David Daney writes:
 > Gary Benson wrote:
 > > +	// Some soon-to-be-removed native code synchronizes on this.
 > > +	loopbackAddress = LOCALHOST;
 > > +      }
 > > +    catch (UnknownHostException e)
 > > +      {
 > > +	throw new RuntimeException("should never happen", e);
 > > +      }
 > >   
 > Would it be better to throw an InternalError here rather than 
 > RuntimeException?  If it truly should never happen then it would be an 
 > InternalError if it did.  If it can happen, then it should be of some 
 > type the describes the problem.

AFAICR this is the kind of thing that happens if the IP address of the
loopback interface doesn't have a resolvable name,

Andrew.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: FYI: Win32 InetAddress fix (really)
  2006-10-03 16:55             ` Andrew Haley
@ 2006-10-03 17:23               ` David Daney
  2006-10-04  8:37                 ` Gary Benson
  0 siblings, 1 reply; 15+ messages in thread
From: David Daney @ 2006-10-03 17:23 UTC (permalink / raw)
  To: Andrew Haley; +Cc: java-patches, gbenson

Andrew Haley wrote:
> David Daney writes:
>  > Gary Benson wrote:
>  > > +	// Some soon-to-be-removed native code synchronizes on this.
>  > > +	loopbackAddress = LOCALHOST;
>  > > +      }
>  > > +    catch (UnknownHostException e)
>  > > +      {
>  > > +	throw new RuntimeException("should never happen", e);
>  > > +      }
>  > >   
>  > Would it be better to throw an InternalError here rather than 
>  > RuntimeException?  If it truly should never happen then it would be an 
>  > InternalError if it did.  If it can happen, then it should be of some 
>  > type the describes the problem.
> 
> AFAICR this is the kind of thing that happens if the IP address of the
> loopback interface doesn't have a resolvable name,
> 

I guess in this specific case it does not really matter as it is in a 
static initializer and the result would be an ExceptionInInitiazerError 
either way.

But in general I don't like the use RuntimeException.  It makes me think 
that we were too lazy to figure out what to do and took the easy (but 
inelegant and probably incorrect) way out by circumventing java's 
checked exception requirements.

David Daney

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: FYI: Win32 InetAddress fix (really)
  2006-10-03 17:23               ` David Daney
@ 2006-10-04  8:37                 ` Gary Benson
  0 siblings, 0 replies; 15+ messages in thread
From: Gary Benson @ 2006-10-04  8:37 UTC (permalink / raw)
  To: java-patches

David Daney wrote:
> Andrew Haley wrote:
> > David Daney writes:
> > > Gary Benson wrote:
> > > > +    catch (UnknownHostException e)
> > > > +      {
> > > > +	throw new RuntimeException("should never happen", e);
> > > > +      }
> > > 
> > > Would it be better to throw an InternalError here rather than
> > > RuntimeException?  If it truly should never happen then it would
> > > be an InternalError if it did.  If it can happen, then it should
> > > be of some type the describes the problem.
> > 
> > AFAICR this is the kind of thing that happens if the IP address of
> > the loopback interface doesn't have a resolvable name,
> 
> I guess in this specific case it does not really matter as
> it is in a static initializer and the result would be an
> ExceptionInInitiazerError either way.
> 
> But in general I don't like the use RuntimeException.  It makes me
> think that we were too lazy to figure out what to do and took the
> easy (but inelegant and probably incorrect) way out by circumventing
> java's checked exception requirements.

The only time getByAddress() throws an UnknownHostException is when
the array of bytes you pass it is is neither 4 or 16 bytes long (for
IPv4 and IPv6 addresses respectively).  And since we're calling it as
getByAddress(new byte[] {127, 0, 0, 1}) it really should never happen.

I specifically didn't use InternalError in this case because its
javadoc says it's thrown to indicate an error in the VM (which I took
to mean the bytecode interpreter) and therefore that InternalError was
something that should only be thrown by the VM.  But, I suppose in
this case the only way this code could be reached is if the VM is
broken, so...

(The ultimate solution would be to have getByAddress throw an
IllegalArgumentException when it's passed an address array of
the wrong length, but that's not something we get to fix :/)

I'll make it InternalError in Classpath so it gets picked up when
I commit the remainder of the InetAddress merge.

Cheers,
Gary

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2006-10-04  8:37 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2006-09-04 14:15 RFC: Untested Win32 InetAddress tweak Gary Benson
2006-09-06 16:58 ` Tom Tromey
2006-09-07  7:36   ` Gary Benson
2006-10-01 17:36 ` Marco Trudel
2006-10-01 18:00   ` Mohan Embar
2006-10-02  7:19     ` Marco Trudel
2006-10-02 10:55       ` FYI: Win32 InetAddress fix Gary Benson
2006-10-02 11:22         ` Marco Trudel
2006-10-02 17:43           ` Tom Tromey
2006-10-03 10:35         ` FYI: Win32 InetAddress fix (really) Gary Benson
2006-10-03 15:10           ` David Daney
2006-10-03 16:55             ` Andrew Haley
2006-10-03 17:23               ` David Daney
2006-10-04  8:37                 ` Gary Benson
2006-10-03 15:30           ` Marco Trudel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).