From 56a1683f894cbc67ec6a6ed46acc04412d9444f0 Mon Sep 17 00:00:00 2001
From: Rob Adams <rob.adams@bigswitch.com>
Date: Thu, 7 Feb 2013 18:03:18 -0800
Subject: [PATCH] Fix a number of issues identified by FindBugs

---
 .../core/OFMessageFilterManager.java          | 16 ++--
 .../core/internal/Controller.java             |  6 +-
 .../core/internal/RoleChanger.java            |  4 +-
 .../core/web/AllSwitchStatisticsResource.java |  8 --
 .../web/AbstractDeviceResource.java           | 11 ++-
 .../firewall/Firewall.java                    |  4 +-
 .../flowcache/FCQueryObj.java                 | 74 +++++++++++--------
 .../loadbalancer/LoadBalancer.java            |  3 +-
 .../floodlightcontroller/packet/BSNPROBE.java | 47 ++++++------
 .../packet/DHCPPacketType.java                |  4 +-
 .../floodlightcontroller/packet/Ethernet.java | 22 ------
 .../packet/LLDPOrganizationalTLV.java         | 42 +++--------
 .../net/floodlightcontroller/packet/TCP.java  |  3 +-
 .../storage/RowOrdering.java                  | 67 ++++++++++++-----
 .../util/LoadMonitor.java                     | 26 ++++---
 .../protocol/vendor/OFBasicVendorId.java      |  2 +-
 .../java/org/openflow/util/HexString.java     |  4 +-
 .../core/internal/OFSwitchImplTest.java       |  1 +
 .../core/util/SingletonTaskTest.java          |  4 +-
 .../internal/DeviceManagerImplTest.java       |  1 -
 .../devicemanager/test/MockDevice.java        | 10 +--
 .../firewall/FirewallTest.java                |  1 -
 .../flowcache/FlowReconcileMgrTest.java       |  1 -
 .../flowcache/PortDownReconciliationTest.java |  1 -
 .../forwarding/ForwardingTest.java            |  1 -
 .../loadbalancer/LoadBalancerTest.java        |  1 -
 .../packet/PacketTest.java                    |  7 +-
 .../storage/tests/StorageTest.java            |  2 +-
 28 files changed, 186 insertions(+), 187 deletions(-)

diff --git a/src/main/java/net/floodlightcontroller/core/OFMessageFilterManager.java b/src/main/java/net/floodlightcontroller/core/OFMessageFilterManager.java
index 391c0026d..fe6be9540 100644
--- a/src/main/java/net/floodlightcontroller/core/OFMessageFilterManager.java
+++ b/src/main/java/net/floodlightcontroller/core/OFMessageFilterManager.java
@@ -69,8 +69,8 @@ public class OFMessageFilterManager
     // The port and client reference for packet streaming
     protected int serverPort = 9090;
     protected final int MaxRetry = 1;
-    protected static TTransport transport = null;
-    protected static PacketStreamer.Client packetClient = null;
+    protected static volatile TTransport transport = null;
+    protected static volatile PacketStreamer.Client packetClient = null;
 
     protected IFloodlightProviderService floodlightProvider = null;
     protected IThreadPoolService threadPool = null;
@@ -112,7 +112,7 @@ public class OFMessageFilterManager
         int i;
 
         if ((filterMap == null) || (filterTimeoutMap == null))
-            return  String.format("%d", FILTER_SETUP_FAILED);
+            return String.format("%s", FILTER_SETUP_FAILED);
 
         for (i=0; i<MAX_FILTERS; ++i) {
             Integer x = prime + i;
@@ -287,15 +287,17 @@ public class OFMessageFilterManager
 
         while (numRetries++ < MaxRetry) {
             try {
-                transport = new TFramedTransport(new TSocket("localhost", 
-                                                             serverPort));
-                transport.open();
+                TFramedTransport t = 
+                        new TFramedTransport(new TSocket("localhost", 
+                                                         serverPort));
+                t.open();
 
-                TProtocol protocol = new  TBinaryProtocol(transport);
+                TProtocol protocol = new  TBinaryProtocol(t);
                 packetClient = new PacketStreamer.Client(protocol);
 
                 log.debug("Have a connection to packetstreamer server " +
                 		  "localhost:{}", serverPort);
+                transport = t;
                 break;
             } catch (TException x) {
                 try {
diff --git a/src/main/java/net/floodlightcontroller/core/internal/Controller.java b/src/main/java/net/floodlightcontroller/core/internal/Controller.java
index f96ac671c..1a41b9bb7 100644
--- a/src/main/java/net/floodlightcontroller/core/internal/Controller.java
+++ b/src/main/java/net/floodlightcontroller/core/internal/Controller.java
@@ -1901,7 +1901,6 @@ public class Controller implements IFloodlightProviderService,
     /**
      * Handle changes to the controller nodes IPs and dispatch update.
      */
-    @SuppressWarnings("unchecked")
     protected void handleControllerNodeIPChanges() {
         HashMap<String,String> curControllerNodeIPs = new HashMap<String,String>();
         HashMap<String,String> addedControllerNodeIPs = new HashMap<String,String>();
@@ -1931,7 +1930,7 @@ public class Controller implements IFloodlightProviderService,
                         // new controller node IP
                         addedControllerNodeIPs.put(controllerID, discoveredIP);
                     }
-                    else if (curIP != discoveredIP) {
+                    else if (!curIP.equals(discoveredIP)) {
                         // IP changed
                         removedControllerNodeIPs.put(controllerID, curIP);
                         addedControllerNodeIPs.put(controllerID, discoveredIP);
@@ -1946,7 +1945,8 @@ public class Controller implements IFloodlightProviderService,
             removedEntries.removeAll(curEntries);
             for (String removedControllerID : removedEntries)
                 removedControllerNodeIPs.put(removedControllerID, controllerNodeIPsCache.get(removedControllerID));
-            controllerNodeIPsCache = (HashMap<String, String>) curControllerNodeIPs.clone();
+            controllerNodeIPsCache.clear();
+            controllerNodeIPsCache.putAll(curControllerNodeIPs);
             HAControllerNodeIPUpdate update = new HAControllerNodeIPUpdate(
                                 curControllerNodeIPs, addedControllerNodeIPs,
                                 removedControllerNodeIPs);
diff --git a/src/main/java/net/floodlightcontroller/core/internal/RoleChanger.java b/src/main/java/net/floodlightcontroller/core/internal/RoleChanger.java
index c90db7a60..9a8f95068 100644
--- a/src/main/java/net/floodlightcontroller/core/internal/RoleChanger.java
+++ b/src/main/java/net/floodlightcontroller/core/internal/RoleChanger.java
@@ -306,7 +306,9 @@ public class RoleChanger {
                     = pendingRequestMap.get(sw);
                 if (pendingList == null) {
                     pendingList = new LinkedList<PendingRoleRequestEntry>();
-                    pendingRequestMap.put(sw, pendingList);
+                    LinkedList<PendingRoleRequestEntry> r = 
+                            pendingRequestMap.putIfAbsent(sw, pendingList);
+                    if (r != null) pendingList = r;
                 }
                 int xid = sendHARoleRequest(sw, role, cookie);
                 PendingRoleRequestEntry entry =
diff --git a/src/main/java/net/floodlightcontroller/core/web/AllSwitchStatisticsResource.java b/src/main/java/net/floodlightcontroller/core/web/AllSwitchStatisticsResource.java
index d012fc8e4..37e6c8402 100644
--- a/src/main/java/net/floodlightcontroller/core/web/AllSwitchStatisticsResource.java
+++ b/src/main/java/net/floodlightcontroller/core/web/AllSwitchStatisticsResource.java
@@ -24,8 +24,6 @@ import java.util.List;
 import java.util.Map;
 
 import net.floodlightcontroller.core.IFloodlightProviderService;
-import net.floodlightcontroller.core.types.MacVlanPair;
-
 import org.openflow.protocol.OFFeaturesReply;
 import org.openflow.protocol.statistics.OFStatistics;
 import org.openflow.protocol.statistics.OFStatisticsType;
@@ -136,7 +134,6 @@ public class AllSwitchStatisticsResource extends SwitchResourceBase {
         private OFStatisticsType statType;
         private REQUESTTYPE requestType;
         private OFFeaturesReply featuresReply;
-        private Map<MacVlanPair, Short> switchTable;
         
         public GetConcurrentStatsThread(long switchId, REQUESTTYPE requestType, OFStatisticsType statType) {
             this.switchId = switchId;
@@ -144,7 +141,6 @@ public class AllSwitchStatisticsResource extends SwitchResourceBase {
             this.statType = statType;
             this.switchReply = null;
             this.featuresReply = null;
-            this.switchTable = null;
         }
         
         public List<OFStatistics> getStatisticsReply() {
@@ -155,10 +151,6 @@ public class AllSwitchStatisticsResource extends SwitchResourceBase {
             return featuresReply;
         }
         
-        public Map<MacVlanPair, Short> getSwitchTable() {
-            return switchTable;
-        }
-        
         public long getSwitchId() {
             return switchId;
         }
diff --git a/src/main/java/net/floodlightcontroller/devicemanager/web/AbstractDeviceResource.java b/src/main/java/net/floodlightcontroller/devicemanager/web/AbstractDeviceResource.java
index 58e79e494..7866b7914 100644
--- a/src/main/java/net/floodlightcontroller/devicemanager/web/AbstractDeviceResource.java
+++ b/src/main/java/net/floodlightcontroller/devicemanager/web/AbstractDeviceResource.java
@@ -156,8 +156,9 @@ public abstract class AbstractDeviceResource extends ServerResource {
                 if (ipv4StartsWith != null) {
                     boolean match = false;
                     for (Integer v : value.getIPv4Addresses()) {
-                        String str = IPv4.fromIPv4Address(v);
+                        String str;
                         if (v != null && 
+                            (str = IPv4.fromIPv4Address(v)) != null &&
                             str.startsWith(ipv4StartsWith)) {
                             match = true;
                             break;
@@ -168,9 +169,10 @@ public abstract class AbstractDeviceResource extends ServerResource {
                 if (dpidStartsWith != null) {
                     boolean match = false;
                     for (SwitchPort v : value.getAttachmentPoints(true)) {
-                        String str = 
-                                HexString.toHexString(v.getSwitchDPID(), 8);
+                        String str;
                         if (v != null && 
+                            (str = HexString.toHexString(v.getSwitchDPID(), 
+                                                         8)) != null &&
                             str.startsWith(dpidStartsWith)) {
                             match = true;
                             break;
@@ -181,8 +183,9 @@ public abstract class AbstractDeviceResource extends ServerResource {
                 if (portStartsWith != null) {
                     boolean match = false;
                     for (SwitchPort v : value.getAttachmentPoints(true)) {
-                        String str = Integer.toString(v.getPort());
+                        String str;
                         if (v != null && 
+                            (str = Integer.toString(v.getPort())) != null &&
                             str.startsWith(portStartsWith)) {
                             match = true;
                             break;
diff --git a/src/main/java/net/floodlightcontroller/firewall/Firewall.java b/src/main/java/net/floodlightcontroller/firewall/Firewall.java
index 6ec91da2e..c6a4fd763 100644
--- a/src/main/java/net/floodlightcontroller/firewall/Firewall.java
+++ b/src/main/java/net/floodlightcontroller/firewall/Firewall.java
@@ -359,9 +359,7 @@ public class Firewall implements IFirewallService, IOFMessageListener,
         // storage, create table and read rules
         storageSource.createTable(TABLE_NAME, null);
         storageSource.setTablePrimaryKeyName(TABLE_NAME, COLUMN_RULEID);
-        synchronized (rules) {
-            this.rules = readRulesFromStorage();
-        }
+        this.rules = readRulesFromStorage();
     }
 
     @Override
diff --git a/src/main/java/net/floodlightcontroller/flowcache/FCQueryObj.java b/src/main/java/net/floodlightcontroller/flowcache/FCQueryObj.java
index 8cad49b31..4e48683cb 100644
--- a/src/main/java/net/floodlightcontroller/flowcache/FCQueryObj.java
+++ b/src/main/java/net/floodlightcontroller/flowcache/FCQueryObj.java
@@ -82,46 +82,58 @@ public class FCQueryObj {
                 + evType + ", callerOpaqueObj=" + callerOpaqueObj + "]";
     }
 
+    @Override
+    public int hashCode() {
+        final int prime = 31;
+        int result = 1;
+        result = prime * result
+                 + ((applInstName == null) ? 0 : applInstName.hashCode());
+        result = prime * result
+                 + ((callerName == null) ? 0 : callerName.hashCode());
+        result = prime
+                 * result
+                 + ((callerOpaqueObj == null) ? 0
+                                             : callerOpaqueObj.hashCode());
+        result = prime * result
+                 + ((dstDevice == null) ? 0 : dstDevice.hashCode());
+        result = prime * result + ((evType == null) ? 0 : evType.hashCode());
+        result = prime
+                 * result
+                 + ((fcQueryHandler == null) ? 0 : fcQueryHandler.hashCode());
+        result = prime * result
+                 + ((srcDevice == null) ? 0 : srcDevice.hashCode());
+        result = prime * result + Arrays.hashCode(vlans);
+        return result;
+    }
+
     @Override
     public boolean equals(Object obj) {
-        if (this == obj)
-            return true;
-        if (obj == null)
-            return false;
-        if (getClass() != obj.getClass())
-            return false;
+        if (this == obj) return true;
+        if (obj == null) return false;
+        if (getClass() != obj.getClass()) return false;
         FCQueryObj other = (FCQueryObj) obj;
         if (applInstName == null) {
-            if (other.applInstName != null)
-                return false;
-        } else if (!applInstName.equals(other.applInstName))
-            return false;
+            if (other.applInstName != null) return false;
+        } else if (!applInstName.equals(other.applInstName)) return false;
         if (callerName == null) {
-            if (other.callerName != null)
-                return false;
-        } else if (!callerName.equals(other.callerName))
-            return false;
+            if (other.callerName != null) return false;
+        } else if (!callerName.equals(other.callerName)) return false;
         if (callerOpaqueObj == null) {
-            if (other.callerOpaqueObj != null)
-                return false;
+            if (other.callerOpaqueObj != null) return false;
         } else if (!callerOpaqueObj.equals(other.callerOpaqueObj))
-            return false;
+                                                                  return false;
         if (dstDevice == null) {
-            if (other.dstDevice != null)
-                return false;
-        } else if (!dstDevice.equals(other.dstDevice))
-            return false;
-        if (evType != other.evType)
-            return false;
-        if (fcQueryHandler != other.fcQueryHandler)
-            return false;
+            if (other.dstDevice != null) return false;
+        } else if (!dstDevice.equals(other.dstDevice)) return false;
+        if (evType != other.evType) return false;
+        if (fcQueryHandler == null) {
+            if (other.fcQueryHandler != null) return false;
+        } else if (!fcQueryHandler.equals(other.fcQueryHandler))
+                                                                return false;
         if (srcDevice == null) {
-            if (other.srcDevice != null)
-                return false;
-        } else if (!srcDevice.equals(other.srcDevice))
-            return false;
-        if (!Arrays.equals(vlans, other.vlans))
-            return false;
+            if (other.srcDevice != null) return false;
+        } else if (!srcDevice.equals(other.srcDevice)) return false;
+        if (!Arrays.equals(vlans, other.vlans)) return false;
         return true;
     }
     
diff --git a/src/main/java/net/floodlightcontroller/loadbalancer/LoadBalancer.java b/src/main/java/net/floodlightcontroller/loadbalancer/LoadBalancer.java
index e67a2fb29..83b9eaf57 100644
--- a/src/main/java/net/floodlightcontroller/loadbalancer/LoadBalancer.java
+++ b/src/main/java/net/floodlightcontroller/loadbalancer/LoadBalancer.java
@@ -668,9 +668,8 @@ public class LoadBalancer implements IFloodlightModule,
     @Override
     public int removePool(String poolId) {
         LBPool pool;
-        pool = pools.get(poolId);
-        
         if(pools!=null){
+            pool = pools.get(poolId);
             if (pool.vipId != null)
                 vips.get(pool.vipId).pools.remove(poolId);
             pools.remove(poolId);
diff --git a/src/main/java/net/floodlightcontroller/packet/BSNPROBE.java b/src/main/java/net/floodlightcontroller/packet/BSNPROBE.java
index 720b45f1c..1343f769b 100644
--- a/src/main/java/net/floodlightcontroller/packet/BSNPROBE.java
+++ b/src/main/java/net/floodlightcontroller/packet/BSNPROBE.java
@@ -143,43 +143,38 @@ public class BSNPROBE extends BasePacket {
         
         return this;
     }
-
-    /* (non-Javadoc)
-     * @see java.lang.Object#hashCode()
-     */
+    
     @Override
     public int hashCode() {
-        final int prime = 883;
+        final int prime = 31;
         int result = super.hashCode();
-        result = prime * result + srcMac.hashCode();
-        result = prime * result + dstMac.hashCode();
-        result = prime * result + (int) (srcSwDpid >> 32) + (int) srcSwDpid;
+        result = prime * result
+                 + (int) (controllerId ^ (controllerId >>> 32));
+        result = prime * result + Arrays.hashCode(dstMac);
+        result = prime * result + sequenceId;
+        result = prime * result + Arrays.hashCode(srcMac);
         result = prime * result + srcPortNo;
+        result = prime * result + (int) (srcSwDpid ^ (srcSwDpid >>> 32));
         return result;
     }
 
-    /* (non-Javadoc)
-     * @see java.lang.Object#equals(java.lang.Object)
-     */
+
     @Override
     public boolean equals(Object obj) {
-        if (this == obj)
-            return true;
-        if (!super.equals(obj))
-            return false;
-        if (!(obj instanceof BSNPROBE))
-            return false;
+        if (this == obj) return true;
+        if (!super.equals(obj)) return false;
+        if (getClass() != obj.getClass()) return false;
         BSNPROBE other = (BSNPROBE) obj;
-        if (!Arrays.equals(srcMac, other.srcMac))
-            return false;
-        if (!Arrays.equals(dstMac, other.dstMac))
-        	return false;
-        return (sequenceId == other.sequenceId &&
-        	    srcSwDpid == other.srcSwDpid &&
-        	    srcPortNo == other.srcPortNo
-        	    );
+        if (controllerId != other.controllerId) return false;
+        if (!Arrays.equals(dstMac, other.dstMac)) return false;
+        if (sequenceId != other.sequenceId) return false;
+        if (!Arrays.equals(srcMac, other.srcMac)) return false;
+        if (srcPortNo != other.srcPortNo) return false;
+        if (srcSwDpid != other.srcSwDpid) return false;
+        return true;
     }
-    
+
+
     public String toString() {
     	StringBuffer sb = new StringBuffer("\n");
     	sb.append("BSN Probe packet");
diff --git a/src/main/java/net/floodlightcontroller/packet/DHCPPacketType.java b/src/main/java/net/floodlightcontroller/packet/DHCPPacketType.java
index 3417a18a2..e90bb147c 100644
--- a/src/main/java/net/floodlightcontroller/packet/DHCPPacketType.java
+++ b/src/main/java/net/floodlightcontroller/packet/DHCPPacketType.java
@@ -77,9 +77,9 @@ public enum DHCPPacketType {
                 return "DHCPLEASEUNKNOWN";
             case 13:
                 return "DHCPLEASEACTIVE";
+            default:
+                return "UNKNOWN";
         }
-        
-        return null;
     }
     public static DHCPPacketType getType(int value) {
         switch (value) {
diff --git a/src/main/java/net/floodlightcontroller/packet/Ethernet.java b/src/main/java/net/floodlightcontroller/packet/Ethernet.java
index 6bd627b0e..44a908118 100644
--- a/src/main/java/net/floodlightcontroller/packet/Ethernet.java
+++ b/src/main/java/net/floodlightcontroller/packet/Ethernet.java
@@ -425,28 +425,6 @@ public class Ethernet extends BasePacket {
             sb.append(p.getDiffServ());
             sb.append("\nnw_proto: ");
             sb.append(p.getProtocol());
-
-            if (pkt instanceof TCP) {
-                sb.append("\ntp_src: ");
-                sb.append(((TCP) pkt).getSourcePort());
-                sb.append("\ntp_dst: ");
-                sb.append(((TCP) pkt).getDestinationPort());
-
-            } else if (pkt instanceof UDP) {
-                sb.append("\ntp_src: ");
-                sb.append(((UDP) pkt).getSourcePort());
-                sb.append("\ntp_dst: ");
-                sb.append(((UDP) pkt).getDestinationPort());
-            }
-
-            if (pkt instanceof ICMP) {
-                ICMP icmp = (ICMP) pkt;
-                sb.append("\nicmp_type: ");
-                sb.append(icmp.getIcmpType());
-                sb.append("\nicmp_code: ");
-                sb.append(icmp.getIcmpCode());
-            }
-
         }
         else if (pkt instanceof DHCP) {
             sb.append("\ndhcp packet");
diff --git a/src/main/java/net/floodlightcontroller/packet/LLDPOrganizationalTLV.java b/src/main/java/net/floodlightcontroller/packet/LLDPOrganizationalTLV.java
index a0930bd0f..830dea605 100644
--- a/src/main/java/net/floodlightcontroller/packet/LLDPOrganizationalTLV.java
+++ b/src/main/java/net/floodlightcontroller/packet/LLDPOrganizationalTLV.java
@@ -139,43 +139,23 @@ public class LLDPOrganizationalTLV extends LLDPTLV {
 
     @Override
     public int hashCode() {
-        final int prime = 1423;
-        int result = 1;
-        result = prime * result + type;
-        result = prime * result + length;
+        final int prime = 31;
+        int result = super.hashCode();
+        result = prime * result + Arrays.hashCode(infoString);
         result = prime * result + Arrays.hashCode(oui);
         result = prime * result + subType;
-        result = prime * result + Arrays.hashCode(infoString);
         return result;
     }
 
     @Override
-    public boolean equals(Object o) {
-        if (o == this) {
-            return true;
-        }
-
-        if (!(o instanceof LLDPOrganizationalTLV)) {
-            return false;
-        }
-
-        LLDPOrganizationalTLV other = (LLDPOrganizationalTLV)o;
-        if (this.type != other.type) {
-            return false;
-        }
-        if (this.length != other.length) {
-            return false;
-        }
-        if (!Arrays.equals(this.oui, other.oui)) {
-            return false;
-        }
-        if (this.subType != other.subType) {
-            return false;
-        }
-        if (!Arrays.equals(this.infoString, other.infoString)) {
-            return false;
-        }
-
+    public boolean equals(Object obj) {
+        if (this == obj) return true;
+        if (!super.equals(obj)) return false;
+        if (getClass() != obj.getClass()) return false;
+        LLDPOrganizationalTLV other = (LLDPOrganizationalTLV) obj;
+        if (!Arrays.equals(infoString, other.infoString)) return false;
+        if (!Arrays.equals(oui, other.oui)) return false;
+        if (subType != other.subType) return false;
         return true;
     }
 }
diff --git a/src/main/java/net/floodlightcontroller/packet/TCP.java b/src/main/java/net/floodlightcontroller/packet/TCP.java
index 06359e4af..c282c5d8a 100644
--- a/src/main/java/net/floodlightcontroller/packet/TCP.java
+++ b/src/main/java/net/floodlightcontroller/packet/TCP.java
@@ -18,6 +18,7 @@
 package net.floodlightcontroller.packet;
 
 import java.nio.ByteBuffer;
+import java.util.Arrays;
 
 /**
  *
@@ -253,7 +254,7 @@ public class TCP extends BasePacket {
                (flags == other.flags) &&
                (windowSize == other.windowSize) &&
                (urgentPointer == other.urgentPointer) &&
-               (dataOffset == 5 || options.equals(other.options));
+               (dataOffset == 5 || Arrays.equals(options,other.options));
     }
 
     @Override
diff --git a/src/main/java/net/floodlightcontroller/storage/RowOrdering.java b/src/main/java/net/floodlightcontroller/storage/RowOrdering.java
index f9e61ed1c..d24e8b1eb 100644
--- a/src/main/java/net/floodlightcontroller/storage/RowOrdering.java
+++ b/src/main/java/net/floodlightcontroller/storage/RowOrdering.java
@@ -43,6 +43,36 @@ public class RowOrdering {
         public Direction getDirection() {
             return direction;
         }
+
+        @Override
+        public int hashCode() {
+            final int prime = 31;
+            int result = 1;
+            result = prime * result + getOuterType().hashCode();
+            result = prime * result
+                     + ((column == null) ? 0 : column.hashCode());
+            result = prime * result
+                     + ((direction == null) ? 0 : direction.hashCode());
+            return result;
+        }
+
+        @Override
+        public boolean equals(Object obj) {
+            if (this == obj) return true;
+            if (obj == null) return false;
+            if (getClass() != obj.getClass()) return false;
+            Item other = (Item) obj;
+            if (!getOuterType().equals(other.getOuterType())) return false;
+            if (column == null) {
+                if (other.column != null) return false;
+            } else if (!column.equals(other.column)) return false;
+            if (direction != other.direction) return false;
+            return true;
+        }
+
+        private RowOrdering getOuterType() {
+            return RowOrdering.this;
+        }
     }
     
     private List<Item> itemList = new ArrayList<Item>();
@@ -96,24 +126,25 @@ public class RowOrdering {
     public List<Item> getItemList() {
         return itemList;
     }
-    
-    public boolean equals(RowOrdering rowOrdering) {
-        if (rowOrdering == null)
-            return false;
-        
-        int len1 = itemList.size();
-        int len2 = rowOrdering.getItemList().size();
-        if (len1 != len2)
-            return false;
-        
-        for (int i = 0; i < len1; i++) {
-            Item item1 = itemList.get(i);
-            Item item2 = rowOrdering.getItemList().get(i);
-            if (!item1.getColumn().equals(item2.getColumn()) ||
-                    item1.getDirection() != item2.getDirection())
-                return false;
-        }
-        
+
+    @Override
+    public int hashCode() {
+        final int prime = 31;
+        int result = 1;
+        result = prime * result
+                 + ((itemList == null) ? 0 : itemList.hashCode());
+        return result;
+    }
+
+    @Override
+    public boolean equals(Object obj) {
+        if (this == obj) return true;
+        if (obj == null) return false;
+        if (getClass() != obj.getClass()) return false;
+        RowOrdering other = (RowOrdering) obj;
+        if (itemList == null) {
+            if (other.itemList != null) return false;
+        } else if (!itemList.equals(other.itemList)) return false;
         return true;
     }
 }
diff --git a/src/main/java/net/floodlightcontroller/util/LoadMonitor.java b/src/main/java/net/floodlightcontroller/util/LoadMonitor.java
index 72bb8a217..5b234cd5e 100644
--- a/src/main/java/net/floodlightcontroller/util/LoadMonitor.java
+++ b/src/main/java/net/floodlightcontroller/util/LoadMonitor.java
@@ -205,17 +205,23 @@ public class LoadMonitor implements Runnable {
 
     protected long readIdle() {
         long idle = 0;
-
+        FileInputStream fs = null;
+        BufferedReader reader = null;
         try {
-            FileInputStream fs = new FileInputStream("/proc/stat");
-            BufferedReader reader =
-                new BufferedReader(new InputStreamReader(fs));
-            idle = Long.parseLong(reader.readLine().split("\\s+")[4]);
-            reader.close();
-            fs.close();
-        }
-        catch (IOException ex) {
-            ex.printStackTrace();
+            try {
+                fs = new FileInputStream("/proc/stat");
+                reader = new BufferedReader(new InputStreamReader(fs));
+                String line = reader.readLine();
+                if (line == null) throw new IOException("Empty file");
+                idle = Long.parseLong(line.split("\\s+")[4]);
+            } finally {
+                if (reader != null)
+                    reader.close();
+                if (fs != null)
+                    fs.close();
+            }
+        } catch (IOException ex) {
+            log.error("Error reading idle time from /proc/stat", ex);
         }
         return idle;
 
diff --git a/src/main/java/org/openflow/protocol/vendor/OFBasicVendorId.java b/src/main/java/org/openflow/protocol/vendor/OFBasicVendorId.java
index 09365d7c5..5f789dc3f 100644
--- a/src/main/java/org/openflow/protocol/vendor/OFBasicVendorId.java
+++ b/src/main/java/org/openflow/protocol/vendor/OFBasicVendorId.java
@@ -87,7 +87,7 @@ public class OFBasicVendorId extends OFVendorId {
      * @return
      */
     public OFVendorDataType lookupVendorDataType(int vendorDataType) {
-        return dataTypeMap.get(vendorDataType);
+        return dataTypeMap.get(Long.valueOf(vendorDataType));
     }
 
     /**
diff --git a/src/main/java/org/openflow/util/HexString.java b/src/main/java/org/openflow/util/HexString.java
index 030823eed..b5628242d 100644
--- a/src/main/java/org/openflow/util/HexString.java
+++ b/src/main/java/org/openflow/util/HexString.java
@@ -48,12 +48,12 @@ public class HexString {
         int i = 0;
         for (; i < (padTo * 2 - arr.length); i++) {
             ret += "0";
-            if ((i % 2) == 1)
+            if ((i % 2) != 0)
                 ret += ":";
         }
         for (int j = 0; j < arr.length; j++) {
             ret += arr[j];
-            if ((((i + j) % 2) == 1) && (j < (arr.length - 1)))
+            if ((((i + j) % 2) != 0) && (j < (arr.length - 1)))
                 ret += ":";
         }
         return ret;        
diff --git a/src/test/java/net/floodlightcontroller/core/internal/OFSwitchImplTest.java b/src/test/java/net/floodlightcontroller/core/internal/OFSwitchImplTest.java
index 4524e72aa..0d6ceb51a 100644
--- a/src/test/java/net/floodlightcontroller/core/internal/OFSwitchImplTest.java
+++ b/src/test/java/net/floodlightcontroller/core/internal/OFSwitchImplTest.java
@@ -29,6 +29,7 @@ public class OFSwitchImplTest extends FloodlightTestCase {
     
     @Before
     public void setUp() throws Exception {
+        super.setUp();
         sw = new OFSwitchImpl();
     }    
     
diff --git a/src/test/java/net/floodlightcontroller/core/util/SingletonTaskTest.java b/src/test/java/net/floodlightcontroller/core/util/SingletonTaskTest.java
index 010c651ac..8203161d9 100644
--- a/src/test/java/net/floodlightcontroller/core/util/SingletonTaskTest.java
+++ b/src/test/java/net/floodlightcontroller/core/util/SingletonTaskTest.java
@@ -33,7 +33,9 @@ public class SingletonTaskTest extends FloodlightTestCase {
     public long time = 0;
     
     @Before
-    public void setup() {
+    public void setUp() throws Exception {
+        super.setUp();
+        
         ran = 0;
         finished = 0;
         time = 0;
diff --git a/src/test/java/net/floodlightcontroller/devicemanager/internal/DeviceManagerImplTest.java b/src/test/java/net/floodlightcontroller/devicemanager/internal/DeviceManagerImplTest.java
index 5e3d764df..385d26826 100644
--- a/src/test/java/net/floodlightcontroller/devicemanager/internal/DeviceManagerImplTest.java
+++ b/src/test/java/net/floodlightcontroller/devicemanager/internal/DeviceManagerImplTest.java
@@ -97,7 +97,6 @@ public class DeviceManagerImplTest extends FloodlightTestCase {
     protected IPacket testARPReqPacket_1, testARPReqPacket_2;
     protected byte[] testARPReplyPacket_1_Srld, testARPReplyPacket_2_Srld;
     private byte[] testARPReplyPacket_3_Serialized;
-    MockFloodlightProvider mockFloodlightProvider;
     DeviceManagerImpl deviceManager;
     MemoryStorageSource storageSource;
     FlowReconcileManager flowReconcileMgr;
diff --git a/src/test/java/net/floodlightcontroller/devicemanager/test/MockDevice.java b/src/test/java/net/floodlightcontroller/devicemanager/test/MockDevice.java
index d05197019..5657cc515 100644
--- a/src/test/java/net/floodlightcontroller/devicemanager/test/MockDevice.java
+++ b/src/test/java/net/floodlightcontroller/devicemanager/test/MockDevice.java
@@ -18,6 +18,7 @@
 package net.floodlightcontroller.devicemanager.test;
 
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collection;
 import java.util.List;
 import java.util.TreeSet;
@@ -81,12 +82,11 @@ public class MockDevice extends Device {
         }
         return vals.toArray(new SwitchPort[vals.size()]);
     }
-    
+
     @Override
     public String toString() {
-        String rv = "MockDevice[entities=+";
-        rv += entities.toString();
-        rv += "]";
-        return rv;
+        return "MockDevice [getEntityClass()=" + getEntityClass()
+               + ", getEntities()=" + Arrays.toString(getEntities()) + "]";
     }
+    
 }
diff --git a/src/test/java/net/floodlightcontroller/firewall/FirewallTest.java b/src/test/java/net/floodlightcontroller/firewall/FirewallTest.java
index 4e8e0f3ab..359f9f4fd 100644
--- a/src/test/java/net/floodlightcontroller/firewall/FirewallTest.java
+++ b/src/test/java/net/floodlightcontroller/firewall/FirewallTest.java
@@ -60,7 +60,6 @@ import org.openflow.util.HexString;
  * @author Amer Tahir
  */
 public class FirewallTest extends FloodlightTestCase {
-    protected MockFloodlightProvider mockFloodlightProvider;
     protected FloodlightContext cntx;
     protected OFPacketIn packetIn;
     protected IOFSwitch sw;
diff --git a/src/test/java/net/floodlightcontroller/flowcache/FlowReconcileMgrTest.java b/src/test/java/net/floodlightcontroller/flowcache/FlowReconcileMgrTest.java
index 0d19e036f..43ae3951c 100644
--- a/src/test/java/net/floodlightcontroller/flowcache/FlowReconcileMgrTest.java
+++ b/src/test/java/net/floodlightcontroller/flowcache/FlowReconcileMgrTest.java
@@ -43,7 +43,6 @@ import org.openflow.protocol.OFType;
 
 public class FlowReconcileMgrTest extends FloodlightTestCase {
 
-    protected MockFloodlightProvider mockFloodlightProvider;
     protected FlowReconcileManager flowReconcileMgr;
     protected MockThreadPoolService threadPool;
     protected ICounterStoreService counterStore;
diff --git a/src/test/java/net/floodlightcontroller/flowcache/PortDownReconciliationTest.java b/src/test/java/net/floodlightcontroller/flowcache/PortDownReconciliationTest.java
index ee4ee0a7e..639136ad5 100644
--- a/src/test/java/net/floodlightcontroller/flowcache/PortDownReconciliationTest.java
+++ b/src/test/java/net/floodlightcontroller/flowcache/PortDownReconciliationTest.java
@@ -83,7 +83,6 @@ import net.floodlightcontroller.topology.ITopologyService;
 
 public class PortDownReconciliationTest extends FloodlightTestCase {
 
-    protected MockFloodlightProvider mockFloodlightProvider;
     protected FloodlightModuleContext fmc;
     protected ILinkDiscoveryService lds;
     protected FlowReconcileManager flowReconcileMgr;
diff --git a/src/test/java/net/floodlightcontroller/forwarding/ForwardingTest.java b/src/test/java/net/floodlightcontroller/forwarding/ForwardingTest.java
index 2d491919a..38be5d828 100644
--- a/src/test/java/net/floodlightcontroller/forwarding/ForwardingTest.java
+++ b/src/test/java/net/floodlightcontroller/forwarding/ForwardingTest.java
@@ -72,7 +72,6 @@ import org.openflow.protocol.action.OFActionOutput;
 import org.openflow.util.HexString;
 
 public class ForwardingTest extends FloodlightTestCase {
-    protected MockFloodlightProvider mockFloodlightProvider;
     protected FloodlightContext cntx;
     protected MockDeviceManager deviceManager;
     protected IRoutingService routingEngine;
diff --git a/src/test/java/net/floodlightcontroller/loadbalancer/LoadBalancerTest.java b/src/test/java/net/floodlightcontroller/loadbalancer/LoadBalancerTest.java
index 95b21f77e..f21fc73d9 100644
--- a/src/test/java/net/floodlightcontroller/loadbalancer/LoadBalancerTest.java
+++ b/src/test/java/net/floodlightcontroller/loadbalancer/LoadBalancerTest.java
@@ -85,7 +85,6 @@ public class LoadBalancerTest extends FloodlightTestCase {
     
     protected FloodlightContext cntx;
     protected FloodlightModuleContext fmc;
-    protected MockFloodlightProvider mockFloodlightProvider;
     protected MockDeviceManager deviceManager;
     protected MockThreadPoolService tps;
     protected FlowReconcileManager frm;
diff --git a/src/test/java/net/floodlightcontroller/packet/PacketTest.java b/src/test/java/net/floodlightcontroller/packet/PacketTest.java
index 3e8aed8ba..b975166fa 100644
--- a/src/test/java/net/floodlightcontroller/packet/PacketTest.java
+++ b/src/test/java/net/floodlightcontroller/packet/PacketTest.java
@@ -17,6 +17,9 @@
 package net.floodlightcontroller.packet;
 
 import static org.junit.Assert.*;
+
+import java.util.Arrays;
+
 import org.junit.Before;
 import org.junit.Test;
 
@@ -121,8 +124,8 @@ public class PacketTest {
             ARP arp = (ARP)pkt;
             ARP newArp = (ARP)newPkt;
             newArp.setSenderProtocolAddress(new byte[] {1,2,3,4});
-            assertEquals(false, newArp.getSenderProtocolAddress()
-                                .equals(arp.getSenderProtocolAddress()));
+            assertEquals(false, Arrays.equals(newArp.getSenderProtocolAddress(),
+                                              arp.getSenderProtocolAddress()));
             assertEquals(false, newPkt.equals(pkt));
         }
         
diff --git a/src/test/java/net/floodlightcontroller/storage/tests/StorageTest.java b/src/test/java/net/floodlightcontroller/storage/tests/StorageTest.java
index 29cc15b70..578cd9d1a 100644
--- a/src/test/java/net/floodlightcontroller/storage/tests/StorageTest.java
+++ b/src/test/java/net/floodlightcontroller/storage/tests/StorageTest.java
@@ -173,7 +173,7 @@ public abstract class StorageTest extends FloodlightTestCase {
                 else if (expectedObject instanceof Double)
                     assertEquals(((Double)expectedObject).doubleValue(), resultSet.getDouble(columnName), 0.00001);
                 else if (expectedObject instanceof byte[])
-                    assertEquals((byte[])expectedObject, resultSet.getByteArray(columnName));
+                    assertTrue(Arrays.equals((byte[])expectedObject, resultSet.getByteArray(columnName)));
                 else if (expectedObject instanceof String)
                     assertEquals((String)expectedObject, resultSet.getString(columnName));
                 else
-- 
GitLab