New diffs. I noticed (because of Bill's earlier comments), that
equals() looks *only* at '_port'. Changed to make hashCode() use
'_port' only.
Index: admin-ee/admin/src/java/com/sun/enterprise/ee/admin/
PortInUse.java
===================================================================
RCS file: /cvs/glassfish/admin-ee/admin/src/java/com/sun/enterprise/
ee/admin/PortInUse.java,v
retrieving revision 1.1.1.1
diff -w -u -r1.1.1.1 PortInUse.java
--- admin-ee/admin/src/java/com/sun/enterprise/ee/admin/
PortInUse.java 8 Aug 2006 19:48:39 -0000 1.1.1.1
+++ admin-ee/admin/src/java/com/sun/enterprise/ee/admin/
PortInUse.java 3 May 2007 22:38:57 -0000
@@ -33,16 +33,17 @@
*
* In addition a conflicting port may optionally be resolved to a
non-conflicting port.
* If this is the case then
+ <b>NOT THREAD SAFE (mutable variables)</b>
*/
-public class PortInUse implements Serializable {
+public final class PortInUse implements Serializable {
public static final int NO_NEW_PORT = -1;
- private String _propertyName = null;
- private int _port;
- private String _hostName = null;
- private String _serverName = null;
- private String _conflictingPropertyName = null;
+ private final String _propertyName;
+ private final int _port;
+ private final String _hostName;
+ private final String _serverName;
+ private final String _conflictingPropertyName;
private int _newPort = NO_NEW_PORT;
/**
@@ -150,13 +151,25 @@
return false;
}
}
- /**
- * this overriding method is temporary added to avoid findBugs
warnings
- * it intentionally not based on port number
- * BTW, I could not find any usage of PortInUse.equals so it
maybe unecessary
+
+ public int hashCode() {
+ return _port;
+
+ /* probably ought to be as follows, but equals() was originally
+ written to use *only* the port.
+
+ private static int
+ hashIt( final Object o ) {
+ return o == null ? 0 : o.hashCode();
+ }
+
+
+ return _port ^ _newPort ^
+ hashIt( _propertyName ) ^
+ hashIt( _hostName ) ^
+ hashIt( _serverName ) ^
+ hashIt( _conflictingPropertyName );
*/
- public int HashCode()
- {
- return ((Object)this).hashCode();
}
+
}
On May 3, 2007, at 12:11 PM, Lloyd L Chambers wrote:
> TIMEOUT: 17:00 PST May 3
> https://glassfish.dev.java.net/issues/show_bug.cgi?id=2965
>
> dpatil: com/sun/enterprise/ee/admin/PortInUse.java:147:150 HE:
> com.sun.enterprise.ee.admin.PortInUse defines equals and uses
> Object.hashCode() (H)
> dpatil: com/sun/enterprise/ee/admin/PortInUse.java:160:160 Nm: VERY
> confusing to have methods
> com.sun.enterprise.ee.admin.PortInUse.HashCode() and
> java.lang.Object.hashCode() (H)
>
>
> Index: admin-ee/admin/src/java/com/sun/enterprise/ee/admin/
> PortInUse.java
> ===================================================================
> RCS file: /cvs/glassfish/admin-ee/admin/src/java/com/sun/enterprise/
> ee/admin/PortInUse.java,v
> retrieving revision 1.1.1.1
> diff -w -u -r1.1.1.1 PortInUse.java
> --- admin-ee/admin/src/java/com/sun/enterprise/ee/admin/
> PortInUse.java 8 Aug 2006 19:48:39 -0000 1.1.1.1
> +++ admin-ee/admin/src/java/com/sun/enterprise/ee/admin/
> PortInUse.java 3 May 2007 18:54:12 -0000
> @@ -33,16 +33,17 @@
> *
> * In addition a conflicting port may optionally be resolved to a
> non-conflicting port.
> * If this is the case then
> + <b>NOT THREAD SAFE (mutable variables)</b>
> */
> -public class PortInUse implements Serializable {
> +public final class PortInUse implements Serializable {
>
> public static final int NO_NEW_PORT = -1;
>
> - private String _propertyName = null;
> - private int _port;
> - private String _hostName = null;
> - private String _serverName = null;
> - private String _conflictingPropertyName = null;
> + private final String _propertyName;
> + private final int _port;
> + private final String _hostName;
> + private final String _serverName;
> + private final String _conflictingPropertyName;
> private int _newPort = NO_NEW_PORT;
> /**
> @@ -150,13 +151,18 @@
> return false;
> }
> }
> - /**
> - * this overriding method is temporary added to avoid findBugs
> warnings
> - * it intentionally not based on port number
> - * BTW, I could not find any usage of PortInUse.equals so it
> maybe unecessary
> - */
> - public int HashCode()
> - {
> - return ((Object)this).hashCode();
> +
> + private static int
> + hashIt( final Object o ) {
> + return o == null ? 0 : o.hashCode();
> }
> +
> + public int hashCode() {
> + return _port ^ _newPort ^
> + hashIt( _propertyName ) ^
> + hashIt( _hostName ) ^
> + hashIt( _serverName ) ^
> + hashIt( _conflictingPropertyName );
> + }
> +
> }
>