Fix some of the suppressed FindBugs warnings: EI_EXPOSE_REP and EI_EXPOSE_REP2
The fix is to copy the data when setting/returning it.
If this becomes a bottleneck, we can revisit the solution.
Change-Id: I7ad345aa8d19a41221b3550dd9d2cfcd08b36fd1
diff --git a/src/main/java/net/onrc/onos/core/packet/ARP.java b/src/main/java/net/onrc/onos/core/packet/ARP.java
index e6fcecc..3b315f3 100644
--- a/src/main/java/net/onrc/onos/core/packet/ARP.java
+++ b/src/main/java/net/onrc/onos/core/packet/ARP.java
@@ -20,8 +20,6 @@
import java.nio.ByteBuffer;
import java.util.Arrays;
-import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
-
/**
* @author David Erickson (daviderickson@cs.stanford.edu)
*/
@@ -123,38 +121,44 @@
/**
* @return the senderHardwareAddress
*/
- @SuppressFBWarnings(value = "EI_EXPOSE_REP",
- justification = "TODO: Return a copy of the object?")
public byte[] getSenderHardwareAddress() {
- return senderHardwareAddress;
+ if (this.senderHardwareAddress == null) {
+ return null;
+ }
+ return this.senderHardwareAddress.clone();
}
/**
* @param senderHardwareAddress the senderHardwareAddress to set
*/
- @SuppressFBWarnings(value = "EI_EXPOSE_REP2",
- justification = "TODO: Store a copy of the object?")
public ARP setSenderHardwareAddress(byte[] senderHardwareAddress) {
- this.senderHardwareAddress = senderHardwareAddress;
+ if (senderHardwareAddress == null) {
+ this.senderHardwareAddress = null;
+ } else {
+ this.senderHardwareAddress = senderHardwareAddress.clone();
+ }
return this;
}
/**
* @return the senderProtocolAddress
*/
- @SuppressFBWarnings(value = "EI_EXPOSE_REP",
- justification = "TODO: Return a copy of the object?")
public byte[] getSenderProtocolAddress() {
- return senderProtocolAddress;
+ if (this.senderProtocolAddress == null) {
+ return null;
+ }
+ return this.senderProtocolAddress.clone();
}
/**
* @param senderProtocolAddress the senderProtocolAddress to set
*/
- @SuppressFBWarnings(value = "EI_EXPOSE_REP2",
- justification = "TODO: Store a copy of the object?")
public ARP setSenderProtocolAddress(byte[] senderProtocolAddress) {
- this.senderProtocolAddress = senderProtocolAddress;
+ if (senderProtocolAddress == null) {
+ this.senderProtocolAddress = null;
+ } else {
+ this.senderProtocolAddress = senderProtocolAddress.clone();
+ }
return this;
}
@@ -166,29 +170,33 @@
/**
* @return the targetHardwareAddress
*/
- @SuppressFBWarnings(value = "EI_EXPOSE_REP",
- justification = "TODO: Return a copy of the object?")
public byte[] getTargetHardwareAddress() {
- return targetHardwareAddress;
+ if (this.targetHardwareAddress == null) {
+ return null;
+ }
+ return this.targetHardwareAddress.clone();
}
/**
* @param targetHardwareAddress the targetHardwareAddress to set
*/
- @SuppressFBWarnings(value = "EI_EXPOSE_REP2",
- justification = "TODO: Store a copy of the object?")
public ARP setTargetHardwareAddress(byte[] targetHardwareAddress) {
- this.targetHardwareAddress = targetHardwareAddress;
+ if (targetHardwareAddress == null) {
+ this.targetHardwareAddress = null;
+ } else {
+ this.targetHardwareAddress = targetHardwareAddress.clone();
+ }
return this;
}
/**
* @return the targetProtocolAddress
*/
- @SuppressFBWarnings(value = "EI_EXPOSE_REP",
- justification = "TODO: Return a copy of the object?")
public byte[] getTargetProtocolAddress() {
- return targetProtocolAddress;
+ if (this.targetProtocolAddress == null) {
+ return null;
+ }
+ return this.targetProtocolAddress.clone();
}
/**
@@ -211,10 +219,12 @@
/**
* @param targetProtocolAddress the targetProtocolAddress to set
*/
- @SuppressFBWarnings(value = "EI_EXPOSE_REP2",
- justification = "TODO: Store a copy of the object?")
public ARP setTargetProtocolAddress(byte[] targetProtocolAddress) {
- this.targetProtocolAddress = targetProtocolAddress;
+ if (targetProtocolAddress == null) {
+ this.targetProtocolAddress = null;
+ } else {
+ this.targetProtocolAddress = targetProtocolAddress.clone();
+ }
return this;
}
diff --git a/src/main/java/net/onrc/onos/core/packet/BSNPROBE.java b/src/main/java/net/onrc/onos/core/packet/BSNPROBE.java
index e8eed9e..03e1666 100644
--- a/src/main/java/net/onrc/onos/core/packet/BSNPROBE.java
+++ b/src/main/java/net/onrc/onos/core/packet/BSNPROBE.java
@@ -22,8 +22,6 @@
import java.nio.ByteBuffer;
import java.util.Arrays;
-import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
-
import org.openflow.util.HexString;
/**
@@ -61,29 +59,35 @@
return this;
}
- @SuppressFBWarnings(value = "EI_EXPOSE_REP",
- justification = "TODO: Return a copy of the object?")
public byte[] getSrcMac() {
- return this.srcMac;
+ if (this.srcMac == null) {
+ return null;
+ }
+ return this.srcMac.clone();
}
- @SuppressFBWarnings(value = "EI_EXPOSE_REP2",
- justification = "TODO: Store a copy of the object?")
public BSNPROBE setSrcMac(byte[] srcMac) {
- this.srcMac = srcMac;
+ if (srcMac == null) {
+ this.srcMac = null;
+ } else {
+ this.srcMac = srcMac.clone();
+ }
return this;
}
- @SuppressFBWarnings(value = "EI_EXPOSE_REP",
- justification = "TODO: Return a copy of the object?")
public byte[] getDstMac() {
- return dstMac;
+ if (this.dstMac == null) {
+ return null;
+ }
+ return this.dstMac.clone();
}
- @SuppressFBWarnings(value = "EI_EXPOSE_REP2",
- justification = "TODO: Store a copy of the object?")
public BSNPROBE setDstMac(byte[] dstMac) {
- this.dstMac = dstMac;
+ if (dstMac == null) {
+ this.dstMac = null;
+ } else {
+ this.dstMac = dstMac.clone();
+ }
return this;
}
diff --git a/src/main/java/net/onrc/onos/core/packet/DHCP.java b/src/main/java/net/onrc/onos/core/packet/DHCP.java
index 9322672..22d6c7a 100644
--- a/src/main/java/net/onrc/onos/core/packet/DHCP.java
+++ b/src/main/java/net/onrc/onos/core/packet/DHCP.java
@@ -23,8 +23,6 @@
import java.util.List;
import java.util.ListIterator;
-import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
-
/**
* @author David Erickson (daviderickson@cs.stanford.edu)
*/
@@ -269,19 +267,22 @@
/**
* @return the clientHardwareAddress
*/
- @SuppressFBWarnings(value = "EI_EXPOSE_REP",
- justification = "TODO: Return a copy of the object?")
public byte[] getClientHardwareAddress() {
- return clientHardwareAddress;
+ if (this.clientHardwareAddress == null) {
+ return null;
+ }
+ return this.clientHardwareAddress.clone();
}
/**
* @param clientHardwareAddress the clientHardwareAddress to set
*/
- @SuppressFBWarnings(value = "EI_EXPOSE_REP2",
- justification = "TODO: Store a copy of the object?")
public DHCP setClientHardwareAddress(byte[] clientHardwareAddress) {
- this.clientHardwareAddress = clientHardwareAddress;
+ if (clientHardwareAddress == null) {
+ this.clientHardwareAddress = null;
+ } else {
+ this.clientHardwareAddress = clientHardwareAddress.clone();
+ }
return this;
}
diff --git a/src/main/java/net/onrc/onos/core/packet/DHCPOption.java b/src/main/java/net/onrc/onos/core/packet/DHCPOption.java
index 81bbed4..d24541e 100644
--- a/src/main/java/net/onrc/onos/core/packet/DHCPOption.java
+++ b/src/main/java/net/onrc/onos/core/packet/DHCPOption.java
@@ -19,8 +19,6 @@
import java.util.Arrays;
-import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
-
/**
* @author David Erickson (daviderickson@cs.stanford.edu)
*/
@@ -62,19 +60,22 @@
/**
* @return the data
*/
- @SuppressFBWarnings(value = "EI_EXPOSE_REP",
- justification = "TODO: Return a copy of the object?")
public byte[] getData() {
- return data;
+ if (this.data == null) {
+ return null;
+ }
+ return this.data.clone();
}
/**
* @param data the data to set
*/
- @SuppressFBWarnings(value = "EI_EXPOSE_REP2",
- justification = "TODO: Store a copy of the object?")
public DHCPOption setData(byte[] data) {
- this.data = data;
+ if (data == null) {
+ this.data = null;
+ } else {
+ this.data = data.clone();
+ }
return this;
}
diff --git a/src/main/java/net/onrc/onos/core/packet/Data.java b/src/main/java/net/onrc/onos/core/packet/Data.java
index 6701965..e79cf7b 100644
--- a/src/main/java/net/onrc/onos/core/packet/Data.java
+++ b/src/main/java/net/onrc/onos/core/packet/Data.java
@@ -19,8 +19,6 @@
import java.util.Arrays;
-import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
-
/**
* @author David Erickson (daviderickson@cs.stanford.edu)
*/
@@ -36,35 +34,41 @@
/**
* @param data
*/
- @SuppressFBWarnings(value = "EI_EXPOSE_REP2",
- justification = "TODO: Store a copy of the object?")
public Data(byte[] data) {
- this.data = data;
+ if (data == null) {
+ this.data = null;
+ } else {
+ this.data = data.clone();
+ }
}
/**
* @return the data
*/
- @SuppressFBWarnings(value = "EI_EXPOSE_REP",
- justification = "TODO: Return a copy of the object?")
public byte[] getData() {
- return data;
+ if (this.data == null) {
+ return null;
+ }
+ return this.data.clone();
}
/**
* @param data the data to set
*/
- @SuppressFBWarnings(value = "EI_EXPOSE_REP2",
- justification = "TODO: Store a copy of the object?")
public Data setData(byte[] data) {
- this.data = data;
+ if (data == null) {
+ this.data = null;
+ } else {
+ this.data = data.clone();
+ }
return this;
}
- @SuppressFBWarnings(value = "EI_EXPOSE_REP",
- justification = "TODO: Return a copy of the object?")
public byte[] serialize() {
- return this.data;
+ if (this.data == null) {
+ return null;
+ }
+ return this.data.clone();
}
@Override
diff --git a/src/main/java/net/onrc/onos/core/packet/IPv4.java b/src/main/java/net/onrc/onos/core/packet/IPv4.java
index ed7d9dc..fbb91ab 100644
--- a/src/main/java/net/onrc/onos/core/packet/IPv4.java
+++ b/src/main/java/net/onrc/onos/core/packet/IPv4.java
@@ -26,8 +26,6 @@
import java.util.HashMap;
import java.util.Map;
-import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
-
/**
* @author David Erickson (daviderickson@cs.stanford.edu)
*/
@@ -267,23 +265,26 @@
/**
* @return the options
*/
- @SuppressFBWarnings(value = "EI_EXPOSE_REP",
- justification = "TODO: Return a copy of the object?")
public byte[] getOptions() {
- return options;
+ if (this.options == null) {
+ return null;
+ }
+ return this.options.clone();
}
/**
* @param options the options to set
*/
- @SuppressFBWarnings(value = "EI_EXPOSE_REP2",
- justification = "TODO: Store a copy of the object?")
public IPv4 setOptions(byte[] options) {
if (options != null && (options.length % 4) > 0) {
throw new IllegalArgumentException(
"Options length must be a multiple of 4");
}
- this.options = options;
+ if (options == null) {
+ this.options = null;
+ } else {
+ this.options = options.clone();
+ }
return this;
}
diff --git a/src/main/java/net/onrc/onos/core/packet/LLDPTLV.java b/src/main/java/net/onrc/onos/core/packet/LLDPTLV.java
index 7acef72..8de0d8a 100644
--- a/src/main/java/net/onrc/onos/core/packet/LLDPTLV.java
+++ b/src/main/java/net/onrc/onos/core/packet/LLDPTLV.java
@@ -20,8 +20,6 @@
import java.nio.ByteBuffer;
import java.util.Arrays;
-import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
-
/**
* @author David Erickson (daviderickson@cs.stanford.edu)
*/
@@ -63,19 +61,22 @@
/**
* @return the value
*/
- @SuppressFBWarnings(value = "EI_EXPOSE_REP",
- justification = "TODO: Return a copy of the object?")
public byte[] getValue() {
- return value;
+ if (this.value == null) {
+ return null;
+ }
+ return this.value.clone();
}
/**
* @param value the value to set
*/
- @SuppressFBWarnings(value = "EI_EXPOSE_REP2",
- justification = "TODO: Store a copy of the object?")
public LLDPTLV setValue(byte[] value) {
- this.value = value;
+ if (value == null) {
+ this.value = null;
+ } else {
+ this.value = value.clone();
+ }
return this;
}
diff --git a/src/main/java/net/onrc/onos/core/packet/TCP.java b/src/main/java/net/onrc/onos/core/packet/TCP.java
index b435d74..def194c 100644
--- a/src/main/java/net/onrc/onos/core/packet/TCP.java
+++ b/src/main/java/net/onrc/onos/core/packet/TCP.java
@@ -20,8 +20,6 @@
import java.nio.ByteBuffer;
import java.util.Arrays;
-import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
-
/**
* @author shudong.zhou@bigswitch.com
*/
@@ -143,17 +141,21 @@
return this;
}
- @SuppressFBWarnings(value = "EI_EXPOSE_REP",
- justification = "TODO: Return a copy of the object?")
public byte[] getOptions() {
- return this.options;
+ if (this.options == null) {
+ return null;
+ }
+ return this.options.clone();
}
- @SuppressFBWarnings(value = "EI_EXPOSE_REP2",
- justification = "TODO: Store a copy of the object?")
public TCP setOptions(final byte[] options) {
- this.options = options;
- this.dataOffset = (byte) ((20 + options.length + 3) >> 2);
+ if (options == null) {
+ this.options = null;
+ this.dataOffset = 0;
+ } else {
+ this.options = options.clone();
+ this.dataOffset = (byte) ((20 + options.length + 3) >> 2);
+ }
return this;
}