From 4e7baadc71201a94de713d73e41b437d27f072aa Mon Sep 17 00:00:00 2001 From: Yang Sun Date: Sun, 14 Apr 2024 16:24:37 +0800 Subject: [PATCH] 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 --- .../MulticastRoutingCoordinatorService.java | 10 ++++---- .../net/module/util/CollectionUtils.java | 24 +++++++++++++++++++ .../net/module/util/CollectionUtilsTest.kt | 17 +++++++++++++ .../MulticastRoutingCoordinatorServiceTest.kt | 7 ++++-- 4 files changed, 52 insertions(+), 6 deletions(-) diff --git a/service/src/com/android/server/connectivity/MulticastRoutingCoordinatorService.java b/service/src/com/android/server/connectivity/MulticastRoutingCoordinatorService.java index 4d5001bf83..ac479b895b 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_CLOEXEC; 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 @@ public class MulticastRoutingCoordinatorService { } 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 @@ public class MulticastRoutingCoordinatorService { 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 @@ public class MulticastRoutingCoordinatorService { @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 @@ public class MulticastRoutingCoordinatorService { 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 39e7ce9456..f3d8c4aeae 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 @@ public final class CollectionUtils { } 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. + * + *

Note this is a linear search, and if multiple keys can map to the same value + * then the smallest index is returned. + * + *

This function compares values with {@code equals} while the + * {@link SparseArray#indexOfValue} compares values using {@code ==}. + */ + public static int getIndexForValue(SparseArray 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 e23f99912e..4ed3afd534 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 @@ class CollectionUtilsTest { CollectionUtils.assoc(listOf(1, 2), list15) } } + + @Test + fun testGetIndexForValue() { + val sparseArray = SparseArray(); + 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 6c2c2560c4..5c994f5b29 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 @@ class MulticastRoutingCoordinatorServiceTest { 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))