홍길동 / onos

Go to a project
Toggle navigation Toggle navigation pinning
Ray Milkey
241b96ac 1 parent 5dfa43f4

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
Inline Side-by-side
Showing 11 changed files with 97 additions and 35 deletions
...@@ -2,6 +2,7 @@ package org.onlab.onos.store.service.impl; ...@@ -2,6 +2,7 @@ package org.onlab.onos.store.service.impl;
2 2
3 import static org.slf4j.LoggerFactory.getLogger; 3 import static org.slf4j.LoggerFactory.getLogger;
4 4
5 +import java.nio.charset.StandardCharsets;
5 import java.util.UUID; 6 import java.util.UUID;
6 import java.util.concurrent.CompletableFuture; 7 import java.util.concurrent.CompletableFuture;
7 import java.util.concurrent.ExecutionException; 8 import java.util.concurrent.ExecutionException;
...@@ -41,7 +42,9 @@ public class DistributedLock implements Lock { ...@@ -41,7 +42,9 @@ public class DistributedLock implements Lock {
41 this.databaseService = databaseService; 42 this.databaseService = databaseService;
42 this.lockManager = lockManager; 43 this.lockManager = lockManager;
43 this.lockId = 44 this.lockId =
44 - (UUID.randomUUID().toString() + "::" + clusterService.getLocalNode().id().toString()).getBytes(); 45 + (UUID.randomUUID().toString() + "::" +
46 + clusterService.getLocalNode().id().toString()).
47 + getBytes(StandardCharsets.UTF_8);
45 } 48 }
46 49
47 @Override 50 @Override
......
...@@ -216,7 +216,7 @@ class RoleManager implements RoleHandler { ...@@ -216,7 +216,7 @@ class RoleManager implements RoleHandler {
216 + "Switch: {}. " 216 + "Switch: {}. "
217 + "This controller has no current role for this sw. " 217 + "This controller has no current role for this sw. "
218 + "Ignoring ...", new Object[] {rri, 218 + "Ignoring ...", new Object[] {rri,
219 - sw.getStringId(), }); 219 + sw == null ? "(null)" : sw.getStringId(), });
220 return RoleRecvStatus.OTHER_EXPECTATION; 220 return RoleRecvStatus.OTHER_EXPECTATION;
221 } 221 }
222 222
......
...@@ -155,7 +155,7 @@ public class OFSwitchImplCPqD13 extends AbstractOpenFlowSwitch { ...@@ -155,7 +155,7 @@ public class OFSwitchImplCPqD13 extends AbstractOpenFlowSwitch {
155 155
156 @Override 156 @Override
157 public void processDriverHandshakeMessage(OFMessage m) { 157 public void processDriverHandshakeMessage(OFMessage m) {
158 - if (!startDriverHandshakeCalled || !startDriverHandshakeCalled) { 158 + if (!startDriverHandshakeCalled) {
159 throw new SwitchDriverSubHandshakeNotStarted(); 159 throw new SwitchDriverSubHandshakeNotStarted();
160 } 160 }
161 if (driverHandshakeComplete.get()) { 161 if (driverHandshakeComplete.get()) {
......
1 -<?xml version="1.0" encoding="UTF-8"?> 1 + <?xml version="1.0" encoding="UTF-8"?>
2 <project xmlns="http://maven.apache.org/POM/4.0.0" 2 <project xmlns="http://maven.apache.org/POM/4.0.0"
3 xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" 3 xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
4 xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd"> 4 xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
...@@ -428,12 +428,6 @@ ...@@ -428,12 +428,6 @@
428 </plugin> 428 </plugin>
429 429
430 <plugin> 430 <plugin>
431 - <groupId>org.codehaus.mojo</groupId>
432 - <artifactId>findbugs-maven-plugin</artifactId>
433 - <version>3.0.0</version>
434 - </plugin>
435 -
436 - <plugin>
437 <groupId>org.apache.felix</groupId> 431 <groupId>org.apache.felix</groupId>
438 <artifactId>maven-scr-plugin</artifactId> 432 <artifactId>maven-scr-plugin</artifactId>
439 <version>1.20.0</version> 433 <version>1.20.0</version>
...@@ -452,6 +446,22 @@ ...@@ -452,6 +446,22 @@
452 </supportedProjectTypes> 446 </supportedProjectTypes>
453 </configuration> 447 </configuration>
454 </plugin> 448 </plugin>
449 + <plugin>
450 + <groupId>org.codehaus.mojo</groupId>
451 + <artifactId>findbugs-maven-plugin</artifactId>
452 + <version>3.0.0</version>
453 + <dependencies>
454 + <dependency>
455 + <groupId>org.onlab.tools</groupId>
456 + <artifactId>onos-build-conf</artifactId>
457 + <version>1.0</version>
458 + </dependency>
459 + </dependencies>
460 + <configuration>
461 + <effort>Max</effort>
462 + <excludeFilterFile>onos/findbugs-suppressions.xml</excludeFilterFile>
463 + </configuration>
464 + </plugin>
455 465
456 <!-- TODO: add findbugs plugin for static code analysis; for explicit invocation only --> 466 <!-- TODO: add findbugs plugin for static code analysis; for explicit invocation only -->
457 <!--This plugin's configuration is used to store Eclipse m2e settings only. It has no influence on the Maven build itself.--> 467 <!--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 @@ ...@@ -586,7 +596,7 @@
586 <artifactId>maven-checkstyle-plugin</artifactId> 596 <artifactId>maven-checkstyle-plugin</artifactId>
587 <version>2.12.1</version> 597 <version>2.12.1</version>
588 <configuration> 598 <configuration>
589 - <configLocation>onos/checkstyle.xml</configLocation> 599 + <!-- <configLocation>onos/checkstyle.xml</configLocation> -->
590 </configuration> 600 </configuration>
591 </plugin> 601 </plugin>
592 602
...@@ -598,10 +608,25 @@ ...@@ -598,10 +608,25 @@
598 <excludes> 608 <excludes>
599 </excludes> 609 </excludes>
600 <rulesets> 610 <rulesets>
601 - <ruleset>onos/pmd.xml</ruleset> 611 + <!-- <ruleset>onos/pmd.xml</ruleset> -->
602 </rulesets> 612 </rulesets>
603 </configuration> 613 </configuration>
604 </plugin> 614 </plugin>
615 + <plugin>
616 + <groupId>org.codehaus.mojo</groupId>
617 + <artifactId>findbugs-maven-plugin</artifactId>
618 + <version>3.0.0</version>
619 + <configuration>
620 + <effort>$Max</effort>
621 + <!-- <excludeFilterFile>${findbugs.excludeFilterFile}</excludeFilterFile> -->
622 + <reportPlugins>
623 + <plugin>
624 + <groupId>org.codehaus.mojo</groupId>
625 + <artifactId>findbugs-maven-plugin</artifactId>
626 + </plugin>
627 + </reportPlugins>
628 + </configuration>
629 + </plugin>
605 </plugins> 630 </plugins>
606 </reporting> 631 </reporting>
607 </project> 632 </project>
......
...@@ -259,10 +259,12 @@ public class FlowModBuilderVer13 extends FlowModBuilder { ...@@ -259,10 +259,12 @@ public class FlowModBuilderVer13 extends FlowModBuilder {
259 ip = (ModIPInstruction) i; 259 ip = (ModIPInstruction) i;
260 ip4 = ip.ip().getIp4Address(); 260 ip4 = ip.ip().getIp4Address();
261 oxm = factory().oxms().ipv4Dst(IPv4Address.of(ip4.toInt())); 261 oxm = factory().oxms().ipv4Dst(IPv4Address.of(ip4.toInt()));
262 + break;
262 case IP_SRC: 263 case IP_SRC:
263 ip = (ModIPInstruction) i; 264 ip = (ModIPInstruction) i;
264 ip4 = ip.ip().getIp4Address(); 265 ip4 = ip.ip().getIp4Address();
265 oxm = factory().oxms().ipv4Src(IPv4Address.of(ip4.toInt())); 266 oxm = factory().oxms().ipv4Src(IPv4Address.of(ip4.toInt()));
267 + break;
266 default: 268 default:
267 log.warn("Unimplemented action type {}.", l3m.subtype()); 269 log.warn("Unimplemented action type {}.", l3m.subtype());
268 break; 270 break;
......
1 +<!--
2 + ~ Copyright 2014 Open Networking Laboratory
3 + ~
4 + ~ Licensed under the Apache License, Version 2.0 (the "License");
5 + ~ you may not use this file except in compliance with the License.
6 + ~ You may obtain a copy of the License at
7 + ~
8 + ~ http://www.apache.org/licenses/LICENSE-2.0
9 + ~
10 + ~ Unless required by applicable law or agreed to in writing, software
11 + ~ distributed under the License is distributed on an "AS IS" BASIS,
12 + ~ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13 + ~ See the License for the specific language governing permissions and
14 + ~ limitations under the License.
15 + -->
16 +<FindBugsFilter>
17 +
18 + <!-- False positives on calls to CompletableFuture methods with a null
19 + parameter -->
20 + <Match>
21 + <Class name="~org\.onlab\.onos\.store\.service\.impl\..*" />
22 + <Bug pattern="NP_NONNULL_PARAM_VIOLATION" />
23 + </Match>
24 +
25 + <!-- Filter out testing application -->
26 + <Match>
27 + <Class name="~org\.onlab\.onos\.foo\..*" />
28 + </Match>
29 +
30 +</FindBugsFilter>
...@@ -40,15 +40,15 @@ public class Ethernet extends BasePacket { ...@@ -40,15 +40,15 @@ public class Ethernet extends BasePacket {
40 public static final short MPLS_UNICAST = (short) 0x8847; 40 public static final short MPLS_UNICAST = (short) 0x8847;
41 public static final short MPLS_MULTICAST = (short) 0x8848; 41 public static final short MPLS_MULTICAST = (short) 0x8848;
42 public static final short DATALAYER_ADDRESS_LENGTH = 6; // bytes 42 public static final short DATALAYER_ADDRESS_LENGTH = 6; // bytes
43 - public static Map<Short, Class<? extends IPacket>> etherTypeClassMap; 43 + public static final Map<Short, Class<? extends IPacket>> ETHER_TYPE_CLASS_MAP =
44 + new HashMap<>();
44 45
45 static { 46 static {
46 - Ethernet.etherTypeClassMap = new HashMap<Short, Class<? extends IPacket>>(); 47 + Ethernet.ETHER_TYPE_CLASS_MAP.put(Ethernet.TYPE_ARP, ARP.class);
47 - Ethernet.etherTypeClassMap.put(Ethernet.TYPE_ARP, ARP.class); 48 + Ethernet.ETHER_TYPE_CLASS_MAP.put(Ethernet.TYPE_RARP, ARP.class);
48 - Ethernet.etherTypeClassMap.put(Ethernet.TYPE_RARP, ARP.class); 49 + Ethernet.ETHER_TYPE_CLASS_MAP.put(Ethernet.TYPE_IPV4, IPv4.class);
49 - Ethernet.etherTypeClassMap.put(Ethernet.TYPE_IPV4, IPv4.class); 50 + Ethernet.ETHER_TYPE_CLASS_MAP.put(Ethernet.TYPE_LLDP, LLDP.class);
50 - Ethernet.etherTypeClassMap.put(Ethernet.TYPE_LLDP, LLDP.class); 51 + Ethernet.ETHER_TYPE_CLASS_MAP.put(Ethernet.TYPE_BSN, LLDP.class);
51 - Ethernet.etherTypeClassMap.put(Ethernet.TYPE_BSN, LLDP.class);
52 } 52 }
53 53
54 protected MacAddress destinationMACAddress; 54 protected MacAddress destinationMACAddress;
...@@ -327,8 +327,8 @@ public class Ethernet extends BasePacket { ...@@ -327,8 +327,8 @@ public class Ethernet extends BasePacket {
327 this.etherType = ethType; 327 this.etherType = ethType;
328 328
329 IPacket payload; 329 IPacket payload;
330 - if (Ethernet.etherTypeClassMap.containsKey(this.etherType)) { 330 + if (Ethernet.ETHER_TYPE_CLASS_MAP.containsKey(this.etherType)) {
331 - final Class<? extends IPacket> clazz = Ethernet.etherTypeClassMap 331 + final Class<? extends IPacket> clazz = Ethernet.ETHER_TYPE_CLASS_MAP
332 .get(this.etherType); 332 .get(this.etherType);
333 try { 333 try {
334 payload = clazz.newInstance(); 334 payload = clazz.newInstance();
......
...@@ -32,10 +32,10 @@ public class IPv4 extends BasePacket { ...@@ -32,10 +32,10 @@ public class IPv4 extends BasePacket {
32 public static final byte PROTOCOL_ICMP = 0x1; 32 public static final byte PROTOCOL_ICMP = 0x1;
33 public static final byte PROTOCOL_TCP = 0x6; 33 public static final byte PROTOCOL_TCP = 0x6;
34 public static final byte PROTOCOL_UDP = 0x11; 34 public static final byte PROTOCOL_UDP = 0x11;
35 - public static Map<Byte, Class<? extends IPacket>> protocolClassMap; 35 + public static Map<Byte, Class<? extends IPacket>> protocolClassMap =
36 + new HashMap<>();
36 37
37 static { 38 static {
38 - IPv4.protocolClassMap = new HashMap<Byte, Class<? extends IPacket>>();
39 IPv4.protocolClassMap.put(IPv4.PROTOCOL_ICMP, ICMP.class); 39 IPv4.protocolClassMap.put(IPv4.PROTOCOL_ICMP, ICMP.class);
40 IPv4.protocolClassMap.put(IPv4.PROTOCOL_TCP, TCP.class); 40 IPv4.protocolClassMap.put(IPv4.PROTOCOL_TCP, TCP.class);
41 IPv4.protocolClassMap.put(IPv4.PROTOCOL_UDP, UDP.class); 41 IPv4.protocolClassMap.put(IPv4.PROTOCOL_UDP, UDP.class);
......
...@@ -16,6 +16,7 @@ ...@@ -16,6 +16,7 @@
16 package org.onlab.packet; 16 package org.onlab.packet;
17 17
18 import java.nio.ByteBuffer; 18 import java.nio.ByteBuffer;
19 +import java.nio.charset.StandardCharsets;
19 import java.util.Arrays; 20 import java.util.Arrays;
20 import java.util.LinkedList; 21 import java.util.LinkedList;
21 import java.util.List; 22 import java.util.List;
...@@ -68,7 +69,7 @@ public class ONLabLddp extends LLDP { ...@@ -68,7 +69,7 @@ public class ONLabLddp extends LLDP {
68 // Contents of full name TLV 69 // Contents of full name TLV
69 private static final byte[] NAME_TLV = ByteBuffer.allocate(NAME_TLV_SIZE + 2) 70 private static final byte[] NAME_TLV = ByteBuffer.allocate(NAME_TLV_SIZE + 2)
70 .putShort(NAME_TLV_HEADER).put(ONLAB_OUI).put(NAME_TLV_SUBTYPE) 71 .putShort(NAME_TLV_HEADER).put(ONLAB_OUI).put(NAME_TLV_SUBTYPE)
71 - .put(OVX_NAME.getBytes()).array(); 72 + .put(OVX_NAME.getBytes(StandardCharsets.UTF_8)).array();
72 73
73 private static final byte DPID_TLV_TYPE = 127; 74 private static final byte DPID_TLV_TYPE = 127;
74 private static final byte DPID_TLV_SIZE = (byte) (12); // 12 = OUI (3) + subtype 75 private static final byte DPID_TLV_SIZE = (byte) (12); // 12 = OUI (3) + subtype
...@@ -203,7 +204,7 @@ public class ONLabLddp extends LLDP { ...@@ -203,7 +204,7 @@ public class ONLabLddp extends LLDP {
203 this.bb = ByteBuffer.wrap(ouiName); 204 this.bb = ByteBuffer.wrap(ouiName);
204 this.bb.put(ONLabLddp.ONLAB_OUI); 205 this.bb.put(ONLabLddp.ONLAB_OUI);
205 this.bb.put(NAME_TLV_SUBTYPE); 206 this.bb.put(NAME_TLV_SUBTYPE);
206 - this.bb.put(name.getBytes()); 207 + this.bb.put(name.getBytes(StandardCharsets.UTF_8));
207 208
208 this.ouiNameTLV.setLength(NAME_TLV_SIZE); 209 this.ouiNameTLV.setLength(NAME_TLV_SIZE);
209 this.ouiNameTLV.setType(NAME_TLV_TYPE); 210 this.ouiNameTLV.setType(NAME_TLV_TYPE);
......
...@@ -19,6 +19,7 @@ import com.google.common.collect.Lists; ...@@ -19,6 +19,7 @@ import com.google.common.collect.Lists;
19 import org.apache.commons.lang.ArrayUtils; 19 import org.apache.commons.lang.ArrayUtils;
20 20
21 import java.nio.ByteBuffer; 21 import java.nio.ByteBuffer;
22 +import java.nio.charset.StandardCharsets;
22 23
23 /** 24 /**
24 * ONOS LLDP containing organizational TLV for ONOS device dicovery. 25 * ONOS LLDP containing organizational TLV for ONOS device dicovery.
...@@ -141,7 +142,7 @@ public class ONOSLLDP extends LLDP { ...@@ -141,7 +142,7 @@ public class ONOSLLDP extends LLDP {
141 public String getNameString() { 142 public String getNameString() {
142 LLDPOrganizationalTLV tlv = getNameTLV(); 143 LLDPOrganizationalTLV tlv = getNameTLV();
143 if (tlv != null) { 144 if (tlv != null) {
144 - return new String(tlv.getInfoString()); 145 + return new String(tlv.getInfoString(), StandardCharsets.UTF_8);
145 } 146 }
146 return null; 147 return null;
147 } 148 }
...@@ -149,7 +150,7 @@ public class ONOSLLDP extends LLDP { ...@@ -149,7 +150,7 @@ public class ONOSLLDP extends LLDP {
149 public String getDeviceString() { 150 public String getDeviceString() {
150 LLDPOrganizationalTLV tlv = getDeviceTLV(); 151 LLDPOrganizationalTLV tlv = getDeviceTLV();
151 if (tlv != null) { 152 if (tlv != null) {
152 - return new String(tlv.getInfoString()); 153 + return new String(tlv.getInfoString(), StandardCharsets.UTF_8);
153 } 154 }
154 return null; 155 return null;
155 } 156 }
......
...@@ -27,18 +27,18 @@ import java.util.Map; ...@@ -27,18 +27,18 @@ import java.util.Map;
27 */ 27 */
28 28
29 public class UDP extends BasePacket { 29 public class UDP extends BasePacket {
30 - public static Map<Short, Class<? extends IPacket>> decodeMap; 30 + public static final Map<Short, Class<? extends IPacket>> DECODE_MAP =
31 + new HashMap<>();
31 public static final short DHCP_SERVER_PORT = (short) 67; 32 public static final short DHCP_SERVER_PORT = (short) 67;
32 public static final short DHCP_CLIENT_PORT = (short) 68; 33 public static final short DHCP_CLIENT_PORT = (short) 68;
33 34
34 static { 35 static {
35 - UDP.decodeMap = new HashMap<Short, Class<? extends IPacket>>();
36 /* 36 /*
37 * Disable DHCP until the deserialize code is hardened to deal with 37 * Disable DHCP until the deserialize code is hardened to deal with
38 * garbage input 38 * garbage input
39 */ 39 */
40 - UDP.decodeMap.put(UDP.DHCP_SERVER_PORT, DHCP.class); 40 + UDP.DECODE_MAP.put(UDP.DHCP_SERVER_PORT, DHCP.class);
41 - UDP.decodeMap.put(UDP.DHCP_CLIENT_PORT, DHCP.class); 41 + UDP.DECODE_MAP.put(UDP.DHCP_CLIENT_PORT, DHCP.class);
42 42
43 } 43 }
44 44
...@@ -231,16 +231,16 @@ public class UDP extends BasePacket { ...@@ -231,16 +231,16 @@ public class UDP extends BasePacket {
231 this.length = bb.getShort(); 231 this.length = bb.getShort();
232 this.checksum = bb.getShort(); 232 this.checksum = bb.getShort();
233 233
234 - if (UDP.decodeMap.containsKey(this.destinationPort)) { 234 + if (UDP.DECODE_MAP.containsKey(this.destinationPort)) {
235 try { 235 try {
236 - this.payload = UDP.decodeMap.get(this.destinationPort) 236 + this.payload = UDP.DECODE_MAP.get(this.destinationPort)
237 .getConstructor().newInstance(); 237 .getConstructor().newInstance();
238 } catch (final Exception e) { 238 } catch (final Exception e) {
239 throw new RuntimeException("Failure instantiating class", e); 239 throw new RuntimeException("Failure instantiating class", e);
240 } 240 }
241 - } else if (UDP.decodeMap.containsKey(this.sourcePort)) { 241 + } else if (UDP.DECODE_MAP.containsKey(this.sourcePort)) {
242 try { 242 try {
243 - this.payload = UDP.decodeMap.get(this.sourcePort) 243 + this.payload = UDP.DECODE_MAP.get(this.sourcePort)
244 .getConstructor().newInstance(); 244 .getConstructor().newInstance();
245 } catch (final Exception e) { 245 } catch (final Exception e) {
246 throw new RuntimeException("Failure instantiating class", e); 246 throw new RuntimeException("Failure instantiating class", e);
......