Incorporating suggestions from review

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/address/AddressRangeImpl.java

@@ -30,8 +30,8 @@ import java.util.Iterator;
 public class AddressRangeImpl implements AddressRange, Serializable {
 	private static final long serialVersionUID = 1;
 
-	private Address minAddress; // mimimum address in this range.
+	private final Address minAddress; // minimum address in this range.
-	private Address maxAddress; // maximum address in this range.
+	private final Address maxAddress; // maximum address in this range.
 
 	/**
 	 * Construct a new AddressRangeImpl from the given range.
@@ -57,11 +57,11 @@ public class AddressRangeImpl implements AddressRange, Serializable {
 			throw new IllegalArgumentException("Start and end addresses are not in the same space.");
 		}
 
-		minAddress = start;
+		if (start.compareTo(end) < 0) {
-		maxAddress = end;
+			minAddress = start;
-
+			maxAddress = end;
-		// swap them if out of order
+		} else {
-		if (minAddress.compareTo(maxAddress) > 0) {
+			// swap them if out of order
 			minAddress = end;
 			maxAddress = start;
 		}
@@ -233,28 +233,28 @@ public class AddressRangeImpl implements AddressRange, Serializable {
 
 	@Override
 	public Iterator<Address> iterator() {
-		return new MyAddressIterator(minAddress, maxAddress);
+		return new MyAddressIterator();
 	}
 
 	private class MyAddressIterator implements Iterator<Address> {
 
 		private Address curr;
-		private Address limit;
 
-		public MyAddressIterator(Address start, Address end) {
+		public MyAddressIterator() {
-			this.curr = start;
+			this.curr = minAddress;
-			this.limit = end;
 		}
 
 		@Override
 		public boolean hasNext() {
-			return curr.add(1).compareTo(limit) <= 0;
+			return curr != null;
 		}
 
 		@Override
 		public Address next() {
 			Address next = curr;
-			curr = curr.add(1);
+			if (curr != null) {
+				curr = curr.equals(maxAddress) ? null : curr.next();
+			}
 			return next;
 		}
 

Ghidra/Framework/SoftwareModeling/src/test/java/ghidra/program/model/address/AddressRangeImplTest.java

@@ -34,14 +34,14 @@ public class AddressRangeImplTest extends AbstractGenericTest {
 		super();
 	}
 
-    @Before
+	@Before
-    public void setUp() throws Exception {
+	public void setUp() throws Exception {
-		
+
 		space = new GenericAddressSpace("xx", 32, AddressSpace.TYPE_RAM, 0);
 	}
 
-@Test
+	@Test
-    public void testIntersect() {
+	public void testIntersect() {
 		AddressRange r2 = new AddressRangeImpl(addr(0), addr(30));
 		AddressRange r1 = new AddressRangeImpl(addr(5), addr(20));
 		AddressRange r3 = r1.intersect(r2);
@@ -63,8 +63,8 @@ public class AddressRangeImplTest extends AbstractGenericTest {
 
 	}
 
-@Test
+	@Test
-    public void testAddressRangeChunker_SameStartAndEndAddress() {
+	public void testAddressRangeChunker_SameStartAndEndAddress() {
 		Address start = addr(0);
 		Address end = addr(0);
 
@@ -79,8 +79,8 @@ public class AddressRangeImplTest extends AbstractGenericTest {
 		assertFalse(it.hasNext());
 	}
 
-@Test
+	@Test
-    public void testAddressRangeChunker_AddressRangeConstructor() {
+	public void testAddressRangeChunker_AddressRangeConstructor() {
 		Address start = addr(0);
 		Address end = addr(2);
 
@@ -105,8 +105,8 @@ public class AddressRangeImplTest extends AbstractGenericTest {
 		assertFalse(it.hasNext());
 	}
 
-@Test
+	@Test
-    public void testAddressRangeChunker_LastRangeSmallerThanChunkSize() {
+	public void testAddressRangeChunker_LastRangeSmallerThanChunkSize() {
 		Address start = addr(0);
 		Address end = addr(30);
 
@@ -134,8 +134,8 @@ public class AddressRangeImplTest extends AbstractGenericTest {
 		assertFalse(it.hasNext());
 	}
 
-@Test
+	@Test
-    public void testAddressRangeChunker_LastRangeEqualToChunkSize() {
+	public void testAddressRangeChunker_LastRangeEqualToChunkSize() {
 		Address start = addr(0);
 		Address end = addr(29);
 
@@ -159,8 +159,8 @@ public class AddressRangeImplTest extends AbstractGenericTest {
 		assertFalse(it.hasNext());
 	}
 
-@Test
+	@Test
-    public void testAddressRangeChunker_NullAddresses() {
+	public void testAddressRangeChunker_NullAddresses() {
 		try {
 			new AddressRangeChunker(null, addr(0), 10);
 			Assert.fail("Did not get exception when passing null address to chunker.");
@@ -178,8 +178,8 @@ public class AddressRangeImplTest extends AbstractGenericTest {
 		}
 	}
 
-@Test
+	@Test
-    public void testAddressRangeChunker_BadChunkSize() {
+	public void testAddressRangeChunker_BadChunkSize() {
 		try {
 			new AddressRangeChunker(addr(0), addr(1), -1);
 			Assert.fail("Did not get exception when passing bad chunk size to chunker.");
@@ -197,8 +197,8 @@ public class AddressRangeImplTest extends AbstractGenericTest {
 		}
 	}
 
-@Test
+	@Test
-    public void testAddressRangeChunker_StartLessThanEnd() {
+	public void testAddressRangeChunker_StartLessThanEnd() {
 		try {
 			new AddressRangeChunker(addr(1), addr(0), 10);
 			Assert.fail("Did not get exception when passing start less than end to chunker.");
@@ -210,8 +210,8 @@ public class AddressRangeImplTest extends AbstractGenericTest {
 		new AddressRangeChunker(addr(0), addr(0), 10); // this is OK
 	}
 
-@Test
+	@Test
-    public void testAddressRangeChunker_DifferentAddressSpaces() {
+	public void testAddressRangeChunker_DifferentAddressSpaces() {
 		AddressSpace space1 = new GenericAddressSpace("xx", 32, AddressSpace.TYPE_RAM, 0);
 		AddressSpace space2 = new GenericAddressSpace("yy", 32, AddressSpace.TYPE_RAM, 0);
 
@@ -221,13 +221,66 @@ public class AddressRangeImplTest extends AbstractGenericTest {
 		try {
 			new AddressRangeChunker(a1, a2, 10);
 			Assert.fail("Did not get exception when passing addresses from different address "
-				+ "spaces to chunker.");
+					+ "spaces to chunker.");
 		}
 		catch (IllegalArgumentException e) {
 			// good!
 		}
 	}
 
+	@Test
+	public void testAddressRange_BoundsOrdering() {
+
+		int size = 15;
+		Address start = addr(5);
+		Address limit = start.add(size);
+
+		AddressRange r1 = new AddressRangeImpl(start, limit);
+		assertTrue(r1.getMinAddress().compareTo(r1.getMaxAddress()) < 0);
+
+		AddressRange r2 = new AddressRangeImpl(limit, start);
+		assertTrue(r2.getMinAddress().compareTo(r2.getMaxAddress()) < 0);
+
+		assertTrue(r1.compareTo(r2) == 0);
+	}
+
+	@Test
+	public void testAddressRangeIteration_RangeEnumeration() {
+		int size = 15;
+		Address start = addr(5);
+		Address limit = start.add(size);
+
+		AddressRange r1 = new AddressRangeImpl(start, limit);
+		int addrCount = 0;
+		Iterator<Address> addrItr = r1.iterator();
+		while (addrItr.hasNext()) {
+			addrItr.next();
+			addrCount++;
+		}
+
+		assertTrue(
+			"Address Iterator does not properly enumerate address range: " +
+					String.format("%s (%d long) -- found %d", r1.toString(), r1.getLength(), addrCount),
+					addrCount == (size + 1));
+	}
+
+	@Test
+	public void testAddressRangeIteration_Extent() {
+		int size = 15;
+		Address start = addr(5);
+		Address limit = start.add(size);
+
+		AddressRange r1 = new AddressRangeImpl(start, limit);
+
+		Iterator<Address> addrItr = r1.iterator();
+		Address lastAddr = Address.NO_ADDRESS;
+		while (addrItr.hasNext()) {
+			lastAddr = addrItr.next();
+		}
+
+		assertTrue("Address Iterator extent does not match end of range", lastAddr.equals(limit));
+	}
+
 	private Address addr(int a) {
 		return new GenericAddress(space, a);
 	}