Use helper function getIndexForValue in MulticastRoutingCoordinator.
The SparseArray.indexOfValue() uses == to compare values,
MulticastRoutingCoordinator uses SparseArray with String type values,
and the indexOfValue function would sometimes return -1 when looking for
a value in the SparseArray.
Added a helper function getIndexForValue in CollectionUtils to get index
by comparing values with equals, and used this function in MulticastRoutingCoordinator.
Updated
MulticastRoutingCoordinatorServiceTest#testMulticastRouting_applyConfigNone_removesMfc
test case to use copies of interface names when updating the multicast
configs, so the test would fail without this fix.
Test: atest NetworkStaticLibTests:com.android.net.moduletests.util.CollectionUtilsTest#testGetIndexForValue
atest android.net.connectivity.com.android.server.connectivity.MulticastRoutingCoordinatorServiceTest#testMulticastRouting_applyConfigNone_removesMfc
Change-Id: I862242d6be01b8552cdd6af6c9691948a9f0bfe0
diff --git a/service/src/com/android/server/connectivity/MulticastRoutingCoordinatorService.java b/service/src/com/android/server/connectivity/MulticastRoutingCoordinatorService.java
index 4d5001b..ac479b8 100644
--- a/service/src/com/android/server/connectivity/MulticastRoutingCoordinatorService.java
+++ b/service/src/com/android/server/connectivity/MulticastRoutingCoordinatorService.java
@@ -27,6 +27,8 @@
import static android.system.OsConstants.SOCK_NONBLOCK;
import static android.system.OsConstants.SOCK_RAW;
+import static com.android.net.module.util.CollectionUtils.getIndexForValue;
+
import android.annotation.NonNull;
import android.annotation.Nullable;
import android.net.MulticastRoutingConfig;
@@ -150,7 +152,7 @@
}
private Integer getInterfaceIndex(String ifName) {
- int mapIndex = mInterfaces.indexOfValue(ifName);
+ int mapIndex = getIndexForValue(mInterfaces, ifName);
if (mapIndex < 0) return null;
return mInterfaces.keyAt(mapIndex);
}
@@ -246,7 +248,7 @@
if (virtualIndex == null) return;
updateMfcs();
- mInterfaces.removeAt(mInterfaces.indexOfValue(ifName));
+ mInterfaces.removeAt(getIndexForValue(mInterfaces, ifName));
mVirtualInterfaces.remove(virtualIndex);
try {
mDependencies.setsockoptMrt6DelMif(mMulticastRoutingFd, virtualIndex);
@@ -270,7 +272,7 @@
@VisibleForTesting
public Integer getVirtualInterfaceIndex(String ifName) {
- int mapIndex = mVirtualInterfaces.indexOfValue(ifName);
+ int mapIndex = getIndexForValue(mVirtualInterfaces, ifName);
if (mapIndex < 0) return null;
return mVirtualInterfaces.keyAt(mapIndex);
}
@@ -291,7 +293,7 @@
private void maybeAddAndTrackInterface(String ifName) {
checkOnHandlerThread();
- if (mVirtualInterfaces.indexOfValue(ifName) >= 0) return;
+ if (getIndexForValue(mVirtualInterfaces, ifName) >= 0) return;
int nextVirtualIndex = getNextAvailableVirtualIndex();
int ifIndex = mDependencies.getInterfaceIndex(ifName);
diff --git a/staticlibs/framework/com/android/net/module/util/CollectionUtils.java b/staticlibs/framework/com/android/net/module/util/CollectionUtils.java
index 39e7ce9..f3d8c4a 100644
--- a/staticlibs/framework/com/android/net/module/util/CollectionUtils.java
+++ b/staticlibs/framework/com/android/net/module/util/CollectionUtils.java
@@ -389,4 +389,28 @@
}
return dest;
}
+
+ /**
+ * Returns an index of the given SparseArray that contains the given value, or -1
+ * number if no keys map to the given value.
+ *
+ * <p>Note this is a linear search, and if multiple keys can map to the same value
+ * then the smallest index is returned.
+ *
+ * <p>This function compares values with {@code equals} while the
+ * {@link SparseArray#indexOfValue} compares values using {@code ==}.
+ */
+ public static <T> int getIndexForValue(SparseArray<T> sparseArray, T value) {
+ for(int i = 0, nsize = sparseArray.size(); i < nsize; i++) {
+ T valueAt = sparseArray.valueAt(i);
+ if (valueAt == null) {
+ if (value == null) {
+ return i;
+ };
+ } else if (valueAt.equals(value)) {
+ return i;
+ }
+ }
+ return -1;
+ }
}
diff --git a/staticlibs/tests/unit/src/com/android/net/module/util/CollectionUtilsTest.kt b/staticlibs/tests/unit/src/com/android/net/module/util/CollectionUtilsTest.kt
index e23f999..4ed3afd 100644
--- a/staticlibs/tests/unit/src/com/android/net/module/util/CollectionUtilsTest.kt
+++ b/staticlibs/tests/unit/src/com/android/net/module/util/CollectionUtilsTest.kt
@@ -16,6 +16,7 @@
package com.android.net.module.util
+import android.util.SparseArray
import androidx.test.filters.SmallTest
import androidx.test.runner.AndroidJUnit4
import com.android.testutils.assertThrows
@@ -179,4 +180,20 @@
CollectionUtils.assoc(listOf(1, 2), list15)
}
}
+
+ @Test
+ fun testGetIndexForValue() {
+ val sparseArray = SparseArray<String>();
+ sparseArray.put(5, "hello");
+ sparseArray.put(10, "abcd");
+ sparseArray.put(20, null);
+
+ val value1 = "abcd";
+ val value1Copy = String(value1.toCharArray())
+ val value2 = null;
+
+ assertEquals(1, CollectionUtils.getIndexForValue(sparseArray, value1));
+ assertEquals(1, CollectionUtils.getIndexForValue(sparseArray, value1Copy));
+ assertEquals(2, CollectionUtils.getIndexForValue(sparseArray, value2));
+ }
}
diff --git a/tests/unit/java/com/android/server/connectivity/MulticastRoutingCoordinatorServiceTest.kt b/tests/unit/java/com/android/server/connectivity/MulticastRoutingCoordinatorServiceTest.kt
index 6c2c256..5c994f5 100644
--- a/tests/unit/java/com/android/server/connectivity/MulticastRoutingCoordinatorServiceTest.kt
+++ b/tests/unit/java/com/android/server/connectivity/MulticastRoutingCoordinatorServiceTest.kt
@@ -402,15 +402,18 @@
mService.getVirtualInterfaceIndex(mIfName1), oifsUpdate)
val mf6cctlDel = createStructMf6cctl(mSourceAddress, mGroupAddressScope5,
mService.getVirtualInterfaceIndex(mIfName1), mEmptyOifs)
+ val ifName1Copy = String(mIfName1.toCharArray())
+ val ifName2Copy = String(mIfName2.toCharArray())
+ val ifName3Copy = String(mIfName3.toCharArray())
verify(mDeps).setsockoptMrt6AddMfc(eq(mFd), eq(mf6cctlAdd))
- applyMulticastForwardNone(mIfName1, mIfName2)
+ applyMulticastForwardNone(ifName1Copy, ifName2Copy)
mLooper.dispatchAll()
verify(mDeps).setsockoptMrt6AddMfc(eq(mFd), eq(mf6cctlUpdate))
- applyMulticastForwardNone(mIfName1, mIfName3)
+ applyMulticastForwardNone(ifName1Copy, ifName3Copy)
mLooper.dispatchAll()
verify(mDeps, timeout(TIMEOUT_MS).times(1)).setsockoptMrt6DelMfc(eq(mFd), eq(mf6cctlDel))