Fix high priority findbugs reported issues
Fixed some code that was reporting findbugs errors
Implemented a suppression mechanism for findbugs
reported errors and a suppression file.
Change-Id: Ie8a2e84cc57ec6ddfa14d782ee89feb095b9dd59
diff --git a/core/store/dist/src/main/java/org/onlab/onos/store/service/impl/DistributedLock.java b/core/store/dist/src/main/java/org/onlab/onos/store/service/impl/DistributedLock.java
index 48a45c4..4998179 100644
--- a/core/store/dist/src/main/java/org/onlab/onos/store/service/impl/DistributedLock.java
+++ b/core/store/dist/src/main/java/org/onlab/onos/store/service/impl/DistributedLock.java
@@ -2,6 +2,7 @@
import static org.slf4j.LoggerFactory.getLogger;
+import java.nio.charset.StandardCharsets;
import java.util.UUID;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.ExecutionException;
@@ -41,7 +42,9 @@
this.databaseService = databaseService;
this.lockManager = lockManager;
this.lockId =
- (UUID.randomUUID().toString() + "::" + clusterService.getLocalNode().id().toString()).getBytes();
+ (UUID.randomUUID().toString() + "::" +
+ clusterService.getLocalNode().id().toString()).
+ getBytes(StandardCharsets.UTF_8);
}
@Override
diff --git a/openflow/ctl/src/main/java/org/onlab/onos/openflow/controller/impl/RoleManager.java b/openflow/ctl/src/main/java/org/onlab/onos/openflow/controller/impl/RoleManager.java
index 734e0ba..51e90e5 100644
--- a/openflow/ctl/src/main/java/org/onlab/onos/openflow/controller/impl/RoleManager.java
+++ b/openflow/ctl/src/main/java/org/onlab/onos/openflow/controller/impl/RoleManager.java
@@ -216,7 +216,7 @@
+ "Switch: {}. "
+ "This controller has no current role for this sw. "
+ "Ignoring ...", new Object[] {rri,
- sw.getStringId(), });
+ sw == null ? "(null)" : sw.getStringId(), });
return RoleRecvStatus.OTHER_EXPECTATION;
}
diff --git a/openflow/ctl/src/main/java/org/onlab/onos/openflow/drivers/impl/OFSwitchImplCPqD13.java b/openflow/ctl/src/main/java/org/onlab/onos/openflow/drivers/impl/OFSwitchImplCPqD13.java
index f5aa0d4..0905f43 100644
--- a/openflow/ctl/src/main/java/org/onlab/onos/openflow/drivers/impl/OFSwitchImplCPqD13.java
+++ b/openflow/ctl/src/main/java/org/onlab/onos/openflow/drivers/impl/OFSwitchImplCPqD13.java
@@ -155,7 +155,7 @@
@Override
public void processDriverHandshakeMessage(OFMessage m) {
- if (!startDriverHandshakeCalled || !startDriverHandshakeCalled) {
+ if (!startDriverHandshakeCalled) {
throw new SwitchDriverSubHandshakeNotStarted();
}
if (driverHandshakeComplete.get()) {
diff --git a/pom.xml b/pom.xml
index 28b387e..99e2e08 100644
--- a/pom.xml
+++ b/pom.xml
@@ -1,4 +1,4 @@
-<?xml version="1.0" encoding="UTF-8"?>
+ <?xml version="1.0" encoding="UTF-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
@@ -428,12 +428,6 @@
</plugin>
<plugin>
- <groupId>org.codehaus.mojo</groupId>
- <artifactId>findbugs-maven-plugin</artifactId>
- <version>3.0.0</version>
- </plugin>
-
- <plugin>
<groupId>org.apache.felix</groupId>
<artifactId>maven-scr-plugin</artifactId>
<version>1.20.0</version>
@@ -452,6 +446,22 @@
</supportedProjectTypes>
</configuration>
</plugin>
+ <plugin>
+ <groupId>org.codehaus.mojo</groupId>
+ <artifactId>findbugs-maven-plugin</artifactId>
+ <version>3.0.0</version>
+ <dependencies>
+ <dependency>
+ <groupId>org.onlab.tools</groupId>
+ <artifactId>onos-build-conf</artifactId>
+ <version>1.0</version>
+ </dependency>
+ </dependencies>
+ <configuration>
+ <effort>Max</effort>
+ <excludeFilterFile>onos/findbugs-suppressions.xml</excludeFilterFile>
+ </configuration>
+ </plugin>
<!-- TODO: add findbugs plugin for static code analysis; for explicit invocation only -->
<!--This plugin's configuration is used to store Eclipse m2e settings only. It has no influence on the Maven build itself.-->
@@ -586,7 +596,7 @@
<artifactId>maven-checkstyle-plugin</artifactId>
<version>2.12.1</version>
<configuration>
- <configLocation>onos/checkstyle.xml</configLocation>
+ <!-- <configLocation>onos/checkstyle.xml</configLocation> -->
</configuration>
</plugin>
@@ -598,10 +608,25 @@
<excludes>
</excludes>
<rulesets>
- <ruleset>onos/pmd.xml</ruleset>
+ <!-- <ruleset>onos/pmd.xml</ruleset> -->
</rulesets>
</configuration>
</plugin>
+ <plugin>
+ <groupId>org.codehaus.mojo</groupId>
+ <artifactId>findbugs-maven-plugin</artifactId>
+ <version>3.0.0</version>
+ <configuration>
+ <effort>$Max</effort>
+ <!-- <excludeFilterFile>${findbugs.excludeFilterFile}</excludeFilterFile> -->
+ <reportPlugins>
+ <plugin>
+ <groupId>org.codehaus.mojo</groupId>
+ <artifactId>findbugs-maven-plugin</artifactId>
+ </plugin>
+ </reportPlugins>
+ </configuration>
+ </plugin>
</plugins>
</reporting>
</project>
diff --git a/providers/openflow/flow/src/main/java/org/onlab/onos/provider/of/flow/impl/FlowModBuilderVer13.java b/providers/openflow/flow/src/main/java/org/onlab/onos/provider/of/flow/impl/FlowModBuilderVer13.java
index f7cf9a5..6f595d0 100644
--- a/providers/openflow/flow/src/main/java/org/onlab/onos/provider/of/flow/impl/FlowModBuilderVer13.java
+++ b/providers/openflow/flow/src/main/java/org/onlab/onos/provider/of/flow/impl/FlowModBuilderVer13.java
@@ -259,10 +259,12 @@
ip = (ModIPInstruction) i;
ip4 = ip.ip().getIp4Address();
oxm = factory().oxms().ipv4Dst(IPv4Address.of(ip4.toInt()));
+ break;
case IP_SRC:
ip = (ModIPInstruction) i;
ip4 = ip.ip().getIp4Address();
oxm = factory().oxms().ipv4Src(IPv4Address.of(ip4.toInt()));
+ break;
default:
log.warn("Unimplemented action type {}.", l3m.subtype());
break;
diff --git a/tools/build/conf/src/main/resources/onos/findbugs-suppressions.xml b/tools/build/conf/src/main/resources/onos/findbugs-suppressions.xml
new file mode 100644
index 0000000..eb5a385
--- /dev/null
+++ b/tools/build/conf/src/main/resources/onos/findbugs-suppressions.xml
@@ -0,0 +1,30 @@
+<!--
+ ~ Copyright 2014 Open Networking Laboratory
+ ~
+ ~ Licensed under the Apache License, Version 2.0 (the "License");
+ ~ you may not use this file except in compliance with the License.
+ ~ You may obtain a copy of the License at
+ ~
+ ~ http://www.apache.org/licenses/LICENSE-2.0
+ ~
+ ~ Unless required by applicable law or agreed to in writing, software
+ ~ distributed under the License is distributed on an "AS IS" BASIS,
+ ~ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ ~ See the License for the specific language governing permissions and
+ ~ limitations under the License.
+ -->
+<FindBugsFilter>
+
+ <!-- False positives on calls to CompletableFuture methods with a null
+ parameter -->
+ <Match>
+ <Class name="~org\.onlab\.onos\.store\.service\.impl\..*" />
+ <Bug pattern="NP_NONNULL_PARAM_VIOLATION" />
+ </Match>
+
+ <!-- Filter out testing application -->
+ <Match>
+ <Class name="~org\.onlab\.onos\.foo\..*" />
+ </Match>
+
+</FindBugsFilter>
diff --git a/utils/misc/src/main/java/org/onlab/packet/Ethernet.java b/utils/misc/src/main/java/org/onlab/packet/Ethernet.java
index 0058e16..f3f61f0 100644
--- a/utils/misc/src/main/java/org/onlab/packet/Ethernet.java
+++ b/utils/misc/src/main/java/org/onlab/packet/Ethernet.java
@@ -40,15 +40,15 @@
public static final short MPLS_UNICAST = (short) 0x8847;
public static final short MPLS_MULTICAST = (short) 0x8848;
public static final short DATALAYER_ADDRESS_LENGTH = 6; // bytes
- public static Map<Short, Class<? extends IPacket>> etherTypeClassMap;
+ public static final Map<Short, Class<? extends IPacket>> ETHER_TYPE_CLASS_MAP =
+ new HashMap<>();
static {
- Ethernet.etherTypeClassMap = new HashMap<Short, Class<? extends IPacket>>();
- Ethernet.etherTypeClassMap.put(Ethernet.TYPE_ARP, ARP.class);
- Ethernet.etherTypeClassMap.put(Ethernet.TYPE_RARP, ARP.class);
- Ethernet.etherTypeClassMap.put(Ethernet.TYPE_IPV4, IPv4.class);
- Ethernet.etherTypeClassMap.put(Ethernet.TYPE_LLDP, LLDP.class);
- Ethernet.etherTypeClassMap.put(Ethernet.TYPE_BSN, LLDP.class);
+ Ethernet.ETHER_TYPE_CLASS_MAP.put(Ethernet.TYPE_ARP, ARP.class);
+ Ethernet.ETHER_TYPE_CLASS_MAP.put(Ethernet.TYPE_RARP, ARP.class);
+ Ethernet.ETHER_TYPE_CLASS_MAP.put(Ethernet.TYPE_IPV4, IPv4.class);
+ Ethernet.ETHER_TYPE_CLASS_MAP.put(Ethernet.TYPE_LLDP, LLDP.class);
+ Ethernet.ETHER_TYPE_CLASS_MAP.put(Ethernet.TYPE_BSN, LLDP.class);
}
protected MacAddress destinationMACAddress;
@@ -327,8 +327,8 @@
this.etherType = ethType;
IPacket payload;
- if (Ethernet.etherTypeClassMap.containsKey(this.etherType)) {
- final Class<? extends IPacket> clazz = Ethernet.etherTypeClassMap
+ if (Ethernet.ETHER_TYPE_CLASS_MAP.containsKey(this.etherType)) {
+ final Class<? extends IPacket> clazz = Ethernet.ETHER_TYPE_CLASS_MAP
.get(this.etherType);
try {
payload = clazz.newInstance();
diff --git a/utils/misc/src/main/java/org/onlab/packet/IPv4.java b/utils/misc/src/main/java/org/onlab/packet/IPv4.java
index e54e9e7..28f6a48 100644
--- a/utils/misc/src/main/java/org/onlab/packet/IPv4.java
+++ b/utils/misc/src/main/java/org/onlab/packet/IPv4.java
@@ -32,10 +32,10 @@
public static final byte PROTOCOL_ICMP = 0x1;
public static final byte PROTOCOL_TCP = 0x6;
public static final byte PROTOCOL_UDP = 0x11;
- public static Map<Byte, Class<? extends IPacket>> protocolClassMap;
+ public static Map<Byte, Class<? extends IPacket>> protocolClassMap =
+ new HashMap<>();
static {
- IPv4.protocolClassMap = new HashMap<Byte, Class<? extends IPacket>>();
IPv4.protocolClassMap.put(IPv4.PROTOCOL_ICMP, ICMP.class);
IPv4.protocolClassMap.put(IPv4.PROTOCOL_TCP, TCP.class);
IPv4.protocolClassMap.put(IPv4.PROTOCOL_UDP, UDP.class);
diff --git a/utils/misc/src/main/java/org/onlab/packet/ONLabLddp.java b/utils/misc/src/main/java/org/onlab/packet/ONLabLddp.java
index 4fd38cd..7978d37 100644
--- a/utils/misc/src/main/java/org/onlab/packet/ONLabLddp.java
+++ b/utils/misc/src/main/java/org/onlab/packet/ONLabLddp.java
@@ -16,6 +16,7 @@
package org.onlab.packet;
import java.nio.ByteBuffer;
+import java.nio.charset.StandardCharsets;
import java.util.Arrays;
import java.util.LinkedList;
import java.util.List;
@@ -68,7 +69,7 @@
// Contents of full name TLV
private static final byte[] NAME_TLV = ByteBuffer.allocate(NAME_TLV_SIZE + 2)
.putShort(NAME_TLV_HEADER).put(ONLAB_OUI).put(NAME_TLV_SUBTYPE)
- .put(OVX_NAME.getBytes()).array();
+ .put(OVX_NAME.getBytes(StandardCharsets.UTF_8)).array();
private static final byte DPID_TLV_TYPE = 127;
private static final byte DPID_TLV_SIZE = (byte) (12); // 12 = OUI (3) + subtype
@@ -203,7 +204,7 @@
this.bb = ByteBuffer.wrap(ouiName);
this.bb.put(ONLabLddp.ONLAB_OUI);
this.bb.put(NAME_TLV_SUBTYPE);
- this.bb.put(name.getBytes());
+ this.bb.put(name.getBytes(StandardCharsets.UTF_8));
this.ouiNameTLV.setLength(NAME_TLV_SIZE);
this.ouiNameTLV.setType(NAME_TLV_TYPE);
diff --git a/utils/misc/src/main/java/org/onlab/packet/ONOSLLDP.java b/utils/misc/src/main/java/org/onlab/packet/ONOSLLDP.java
index 0b33c45..5b3902a 100644
--- a/utils/misc/src/main/java/org/onlab/packet/ONOSLLDP.java
+++ b/utils/misc/src/main/java/org/onlab/packet/ONOSLLDP.java
@@ -19,6 +19,7 @@
import org.apache.commons.lang.ArrayUtils;
import java.nio.ByteBuffer;
+import java.nio.charset.StandardCharsets;
/**
* ONOS LLDP containing organizational TLV for ONOS device dicovery.
@@ -141,7 +142,7 @@
public String getNameString() {
LLDPOrganizationalTLV tlv = getNameTLV();
if (tlv != null) {
- return new String(tlv.getInfoString());
+ return new String(tlv.getInfoString(), StandardCharsets.UTF_8);
}
return null;
}
@@ -149,7 +150,7 @@
public String getDeviceString() {
LLDPOrganizationalTLV tlv = getDeviceTLV();
if (tlv != null) {
- return new String(tlv.getInfoString());
+ return new String(tlv.getInfoString(), StandardCharsets.UTF_8);
}
return null;
}
diff --git a/utils/misc/src/main/java/org/onlab/packet/UDP.java b/utils/misc/src/main/java/org/onlab/packet/UDP.java
index 73a196a..19e23c0 100644
--- a/utils/misc/src/main/java/org/onlab/packet/UDP.java
+++ b/utils/misc/src/main/java/org/onlab/packet/UDP.java
@@ -27,18 +27,18 @@
*/
public class UDP extends BasePacket {
- public static Map<Short, Class<? extends IPacket>> decodeMap;
+ public static final Map<Short, Class<? extends IPacket>> DECODE_MAP =
+ new HashMap<>();
public static final short DHCP_SERVER_PORT = (short) 67;
public static final short DHCP_CLIENT_PORT = (short) 68;
static {
- UDP.decodeMap = new HashMap<Short, Class<? extends IPacket>>();
/*
* Disable DHCP until the deserialize code is hardened to deal with
* garbage input
*/
- UDP.decodeMap.put(UDP.DHCP_SERVER_PORT, DHCP.class);
- UDP.decodeMap.put(UDP.DHCP_CLIENT_PORT, DHCP.class);
+ UDP.DECODE_MAP.put(UDP.DHCP_SERVER_PORT, DHCP.class);
+ UDP.DECODE_MAP.put(UDP.DHCP_CLIENT_PORT, DHCP.class);
}
@@ -231,16 +231,16 @@
this.length = bb.getShort();
this.checksum = bb.getShort();
- if (UDP.decodeMap.containsKey(this.destinationPort)) {
+ if (UDP.DECODE_MAP.containsKey(this.destinationPort)) {
try {
- this.payload = UDP.decodeMap.get(this.destinationPort)
+ this.payload = UDP.DECODE_MAP.get(this.destinationPort)
.getConstructor().newInstance();
} catch (final Exception e) {
throw new RuntimeException("Failure instantiating class", e);
}
- } else if (UDP.decodeMap.containsKey(this.sourcePort)) {
+ } else if (UDP.DECODE_MAP.containsKey(this.sourcePort)) {
try {
- this.payload = UDP.decodeMap.get(this.sourcePort)
+ this.payload = UDP.DECODE_MAP.get(this.sourcePort)
.getConstructor().newInstance();
} catch (final Exception e) {
throw new RuntimeException("Failure instantiating class", e);