Address review comments
diff --git a/java_gen/pre-written/src/main/java/org/projectfloodlight/openflow/types/IPAddress.java b/java_gen/pre-written/src/main/java/org/projectfloodlight/openflow/types/IPAddress.java
index 54465bf..c96be83 100644
--- a/java_gen/pre-written/src/main/java/org/projectfloodlight/openflow/types/IPAddress.java
+++ b/java_gen/pre-written/src/main/java/org/projectfloodlight/openflow/types/IPAddress.java
@@ -10,14 +10,13 @@
* @return true if this represents a valid CIDR style netmask, false
* otherwise
*/
- public boolean isCidrMask() {
- return asCidrMaskLength() != -1;
- }
+ public abstract boolean isCidrMask();
/**
* If this IPAddress represents a valid CIDR style netmask (see
* isCidrMask()) returns the length of the prefix (the number of "1" bits).
- * @return length of CIDR mask or -1 if this is not a CIDR netmask
+ * @return length of CIDR mask if this represents a valid CIDR mask
+ * @throws IllegalStateException if isCidrMask() == false
*/
public abstract int asCidrMaskLength();
diff --git a/java_gen/pre-written/src/main/java/org/projectfloodlight/openflow/types/IPAddressWithMask.java b/java_gen/pre-written/src/main/java/org/projectfloodlight/openflow/types/IPAddressWithMask.java
index 654e72c..2087ab4 100644
--- a/java_gen/pre-written/src/main/java/org/projectfloodlight/openflow/types/IPAddressWithMask.java
+++ b/java_gen/pre-written/src/main/java/org/projectfloodlight/openflow/types/IPAddressWithMask.java
@@ -27,7 +27,7 @@
res.append(value.toString());
res.append('/');
- if (mask.asCidrMaskLength() != -1) {
+ if (mask.isCidrMask()) {
// CIDR notation
res.append(mask.asCidrMaskLength());
} else {
diff --git a/java_gen/pre-written/src/main/java/org/projectfloodlight/openflow/types/IPv4Address.java b/java_gen/pre-written/src/main/java/org/projectfloodlight/openflow/types/IPv4Address.java
index 87167fb..2505d56 100644
--- a/java_gen/pre-written/src/main/java/org/projectfloodlight/openflow/types/IPv4Address.java
+++ b/java_gen/pre-written/src/main/java/org/projectfloodlight/openflow/types/IPv4Address.java
@@ -18,6 +18,11 @@
static final int LENGTH = 4;
private final int rawValue;
+ private static int NOT_A_CIDR_MASK = -1;
+ private static int CIDR_MASK_CACHE_UNSET = -2;
+ // Must appear before the static IPv4Address constant assignments
+ private volatile int cidrMaskLengthCache = CIDR_MASK_CACHE_UNSET;
+
private final static int NONE_VAL = 0x0;
public final static IPv4Address NONE = new IPv4Address(NONE_VAL);
@@ -33,18 +38,34 @@
return IPVersion.IPv4;
}
+ private int asCidrMaskLengthInternal() {
+ if (cidrMaskLengthCache == CIDR_MASK_CACHE_UNSET) {
+ // No lock required. We only write cidrMaskLengthCache once
+ int maskint = getInt();
+ if (maskint == 0) {
+ cidrMaskLengthCache = 0;
+ } else if (Integer.bitCount((~maskint) + 1) == 1) {
+ // IP represents a true CIDR prefix length
+ cidrMaskLengthCache = Integer.bitCount(maskint);
+ } else {
+ cidrMaskLengthCache = NOT_A_CIDR_MASK;
+ }
+ }
+ return cidrMaskLengthCache;
+ }
+
+ @Override
+ public boolean isCidrMask() {
+ return asCidrMaskLengthInternal() != NOT_A_CIDR_MASK;
+ }
@Override
public int asCidrMaskLength() {
- int maskint = getInt();
- if (maskint == 0)
- return 0;
- else if (Integer.bitCount((~maskint) + 1) == 1) {
- // IP represents a true CIDR prefix length
- return Integer.bitCount(maskint);
+ if (!isCidrMask()) {
+ throw new IllegalStateException("IP is not a valid CIDR prefix " +
+ "mask " + toString());
} else {
- // IP is not a true prefix.
- return -1;
+ return asCidrMaskLengthInternal();
}
}
diff --git a/java_gen/pre-written/src/main/java/org/projectfloodlight/openflow/types/IPv6Address.java b/java_gen/pre-written/src/main/java/org/projectfloodlight/openflow/types/IPv6Address.java
index 63ae700..b69035c 100644
--- a/java_gen/pre-written/src/main/java/org/projectfloodlight/openflow/types/IPv6Address.java
+++ b/java_gen/pre-written/src/main/java/org/projectfloodlight/openflow/types/IPv6Address.java
@@ -1,11 +1,9 @@
package org.projectfloodlight.openflow.types;
-import java.math.BigInteger;
import java.util.regex.Pattern;
import org.jboss.netty.buffer.ChannelBuffer;
import org.projectfloodlight.openflow.exceptions.OFParseError;
-
import com.google.common.hash.PrimitiveSink;
import com.google.common.primitives.Longs;
@@ -20,10 +18,16 @@
private final long raw1;
private final long raw2;
+ private static int NOT_A_CIDR_MASK = -1;
+ private static int CIDR_MASK_CACHE_UNSET = -2;
+ // Must appear before the static IPv4Address constant assignments
+ private volatile int cidrMaskLengthCache = CIDR_MASK_CACHE_UNSET;
+
private final static long NONE_VAL1 = 0x0L;
private final static long NONE_VAL2 = 0x0L;
public static final IPv6Address NONE = new IPv6Address(NONE_VAL1, NONE_VAL2);
+
public static final IPv6Address NO_MASK = IPv6Address.of(0xFFFFFFFFFFFFFFFFl, 0xFFFFFFFFFFFFFFFFl);
public static final IPv6Address FULL_MASK = IPv6Address.of(0x0, 0x0);
@@ -38,17 +42,52 @@
}
+ private int computeCidrMask64(long raw) {
+ long mask = raw;
+ if (raw == 0)
+ return 0;
+ else if (Long.bitCount((~mask) + 1L) == 1L) {
+ // represent a true CIDR prefix length
+ return Long.bitCount(mask);
+ }
+ else {
+ // Not a true prefix
+ return NOT_A_CIDR_MASK;
+ }
+ }
+
+ private int asCidrMaskLengthInternal() {
+ if (cidrMaskLengthCache == CIDR_MASK_CACHE_UNSET) {
+ synchronized (this) {
+ if (raw1 == 0 && raw2 == 0) {
+ cidrMaskLengthCache = 0;
+ } else if (raw1 == -1) {
+ // top half is all 1 bits
+ cidrMaskLengthCache = computeCidrMask64(raw2);
+ if (cidrMaskLengthCache != NOT_A_CIDR_MASK)
+ cidrMaskLengthCache += 64;
+ } else if (raw2 == 0) {
+ cidrMaskLengthCache = computeCidrMask64(raw1);
+ } else {
+ cidrMaskLengthCache = NOT_A_CIDR_MASK;
+ }
+ }
+ }
+ return cidrMaskLengthCache;
+ }
+
+ @Override
+ public boolean isCidrMask() {
+ return asCidrMaskLengthInternal() != NOT_A_CIDR_MASK;
+ }
+
@Override
public int asCidrMaskLength() {
- BigInteger maskBigint = new BigInteger(getBytes());
- if (maskBigint.equals(BigInteger.ZERO))
- return 0; // Thanks, signed BigInteger
- else if (maskBigint.not().add(BigInteger.ONE).bitCount() == 1) {
- // Need to get a positive BigInteger before we can count
- return new BigInteger(1, getBytes()).bitCount();
+ if (!isCidrMask()) {
+ throw new IllegalStateException("IP is not a valid CIDR prefix " +
+ "mask " + toString());
} else {
- // IP is not a true prefix.
- return -1;
+ return asCidrMaskLengthInternal();
}
}
diff --git a/java_gen/pre-written/src/test/java/org/projectfloodlight/openflow/types/IPv4AddressTest.java b/java_gen/pre-written/src/test/java/org/projectfloodlight/openflow/types/IPv4AddressTest.java
index 975c7d3..065331f 100644
--- a/java_gen/pre-written/src/test/java/org/projectfloodlight/openflow/types/IPv4AddressTest.java
+++ b/java_gen/pre-written/src/test/java/org/projectfloodlight/openflow/types/IPv4AddressTest.java
@@ -1,10 +1,6 @@
package org.projectfloodlight.openflow.types;
-import static org.junit.Assert.assertArrayEquals;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertThat;
-import static org.junit.Assert.fail;
+import static org.junit.Assert.*;
import org.hamcrest.CoreMatchers;
import org.jboss.netty.buffer.ChannelBuffers;
@@ -102,6 +98,35 @@
@Test
+ public void testConstants() {
+ byte[] zeros = { (byte)0x00, (byte)0x00, (byte)0x00, (byte)0x00 };
+ byte[] ones = { (byte)0xFF, (byte)0xFF, (byte)0xFF, (byte)0xFF };
+ // Make sure class initializtation and static assignment don't get
+ // messed up. Test everything twice for cached values
+ assertTrue(IPv4Address.NONE.isCidrMask());
+ assertEquals(0, IPv4Address.NONE.asCidrMaskLength());
+ assertArrayEquals(zeros, IPv4Address.NONE.getBytes());
+ assertTrue(IPv4Address.NONE.isCidrMask());
+ assertEquals(0, IPv4Address.NONE.asCidrMaskLength());
+ assertArrayEquals(zeros, IPv4Address.NONE.getBytes());
+
+ assertTrue(IPv4Address.NO_MASK.isCidrMask());
+ assertEquals(32, IPv4Address.NO_MASK.asCidrMaskLength());
+ assertArrayEquals(ones, IPv4Address.NO_MASK.getBytes());
+ assertTrue(IPv4Address.NO_MASK.isCidrMask());
+ assertEquals(32, IPv4Address.NO_MASK.asCidrMaskLength());
+ assertArrayEquals(ones, IPv4Address.NO_MASK.getBytes());
+
+ assertTrue(IPv4Address.FULL_MASK.isCidrMask());
+ assertEquals(0, IPv4Address.FULL_MASK.asCidrMaskLength());
+ assertArrayEquals(zeros, IPv4Address.FULL_MASK.getBytes());
+ assertTrue(IPv4Address.FULL_MASK.isCidrMask());
+ assertEquals(0, IPv4Address.FULL_MASK.asCidrMaskLength());
+ assertArrayEquals(zeros, IPv4Address.FULL_MASK.getBytes());
+ }
+
+
+ @Test
public void testOfString() {
for(int i=0; i < testAddresses.length; i++ ) {
IPv4Address ip = IPv4Address.of(testStrings[i]);
@@ -153,7 +178,18 @@
assertArrayEquals(ipsWithMaskValues[i][0], ip.getBytes());
}
IPv4Address mask = value.getMask();
- assertEquals(ipsWithMaskLengths[i], value.getMask().asCidrMaskLength());
+ if (ipsWithMaskLengths[i] == -1) {
+ assertFalse(mask.isCidrMask());
+ try {
+ mask.asCidrMaskLength();
+ fail("Expected IllegalStateException not thrown");
+ } catch(IllegalStateException e) {
+ //expected
+ }
+ } else {
+ assertTrue(mask.isCidrMask());
+ assertEquals(ipsWithMaskLengths[i], mask.asCidrMaskLength());
+ }
assertArrayEquals(ipsWithMaskValues[i][1], mask.getBytes());
byte[] ipBytes = new byte[4];
System.arraycopy(ipsWithMaskValues[i][0], 0, ipBytes, 0, 4);
diff --git a/java_gen/pre-written/src/test/java/org/projectfloodlight/openflow/types/IPv6AddressTest.java b/java_gen/pre-written/src/test/java/org/projectfloodlight/openflow/types/IPv6AddressTest.java
index 706fb25..a008070 100644
--- a/java_gen/pre-written/src/test/java/org/projectfloodlight/openflow/types/IPv6AddressTest.java
+++ b/java_gen/pre-written/src/test/java/org/projectfloodlight/openflow/types/IPv6AddressTest.java
@@ -1,10 +1,6 @@
package org.projectfloodlight.openflow.types;
-import static org.junit.Assert.assertArrayEquals;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertThat;
-import static org.junit.Assert.fail;
+import static org.junit.Assert.*;
import java.net.Inet6Address;
import java.net.InetAddress;
@@ -71,6 +67,40 @@
};
@Test
+ public void testConstants() {
+ byte[] zeros = { (byte)0x00, (byte)0x00, (byte)0x00, (byte)0x00,
+ (byte)0x00, (byte)0x00, (byte)0x00, (byte)0x00,
+ (byte)0x00, (byte)0x00, (byte)0x00, (byte)0x00,
+ (byte)0x00, (byte)0x00, (byte)0x00, (byte)0x00 };
+ byte[] ones = { (byte)0xFF, (byte)0xFF, (byte)0xFF, (byte)0xFF,
+ (byte)0xFF, (byte)0xFF, (byte)0xFF, (byte)0xFF,
+ (byte)0xFF, (byte)0xFF, (byte)0xFF, (byte)0xFF,
+ (byte)0xFF, (byte)0xFF, (byte)0xFF, (byte)0xFF };
+ // Make sure class initializtation and static assignment don't get
+ // messed up. Test everything twice for cached values
+ assertTrue(IPv6Address.NONE.isCidrMask());
+ assertEquals(0, IPv6Address.NONE.asCidrMaskLength());
+ assertArrayEquals(zeros, IPv6Address.NONE.getBytes());
+ assertTrue(IPv6Address.NONE.isCidrMask());
+ assertEquals(0, IPv6Address.NONE.asCidrMaskLength());
+ assertArrayEquals(zeros, IPv6Address.NONE.getBytes());
+
+ assertTrue(IPv6Address.NO_MASK.isCidrMask());
+ assertEquals(128, IPv6Address.NO_MASK.asCidrMaskLength());
+ assertArrayEquals(ones, IPv6Address.NO_MASK.getBytes());
+ assertTrue(IPv6Address.NO_MASK.isCidrMask());
+ assertEquals(128, IPv6Address.NO_MASK.asCidrMaskLength());
+ assertArrayEquals(ones, IPv6Address.NO_MASK.getBytes());
+
+ assertTrue(IPv6Address.FULL_MASK.isCidrMask());
+ assertEquals(0, IPv6Address.FULL_MASK.asCidrMaskLength());
+ assertArrayEquals(zeros, IPv6Address.FULL_MASK.getBytes());
+ assertTrue(IPv6Address.FULL_MASK.isCidrMask());
+ assertEquals(0, IPv6Address.FULL_MASK.asCidrMaskLength());
+ assertArrayEquals(zeros, IPv6Address.FULL_MASK.getBytes());
+ }
+
+ @Test
public void testMasked() throws UnknownHostException {
for(WithMaskTaskCase w: withMasks) {
IPv6AddressWithMask value = IPv6AddressWithMask.of(w.input);
@@ -83,7 +113,19 @@
}
InetAddress inetAddress = InetAddress.getByName(w.input.split("/")[0]);
- assertEquals("Input " + w.input, w.expectedMaskLength, value.getMask().asCidrMaskLength());
+ if (w.expectedMaskLength == -1) {
+ assertFalse(value.getMask().isCidrMask());
+ try {
+ value.getMask().asCidrMaskLength();
+ fail("Expected IllegalStateException not thrown");
+ } catch(IllegalStateException e) {
+ //expected
+ }
+ } else {
+ assertTrue(value.getMask().isCidrMask());
+ assertEquals("Input " + w.input, w.expectedMaskLength,
+ value.getMask().asCidrMaskLength());
+ }
byte[] address = inetAddress.getAddress();
assertEquals(address.length, value.getValue().getBytes().length);