> + /* Insert more than UID_GID_MAP_MAX_BASE_EXTENTS (e.g., 10) elements */
> + for (i = 0; i < 10; i++) {
> +   extent.first = i * 10;
> +   extent.lower_first = i * 100;

Both .first and .lower_first increase monotonically with i. So
forward[] is already sorted by .first and reverse[] is already sorted
by .lower_first before sort_idmaps() runs. The sort is a no op on
this input.

> + /* Verify sorting is correct */
> + for (i = 0; i < map.nr_extents; i++) {
> +   KUNIT_EXPECT_EQ(test, map.forward[i].first, i * 10);
> +   KUNIT_EXPECT_EQ(test, map.reverse[i].lower_first, i * 100);

This passes even if sort_idmaps() did nothing. The test cannot tell
a working sort from a broken one.

To actually verify sorting, insert the extents out of order so that
sort_idmaps() has to rearrange them. For example:

    extent.first = (9 - i) * 10;
    extent.lower_first = (9 - i) * 100;

Then after sort_idmaps(), forward[0].first should be 0, forward[1]
should be 10, etc. That proves the sort moved things.

Nits:

> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, map.forward);

Redundant. insert_extent() just allocated it on the 6th call, and
the previous KUNIT_EXPECT_EQ(test, ret, 0) already confirmed success.
If you want to keep it, at least put it after the loop where it
protects the sort, not inside the insertion loop where it tests
something that just succeeded.

> + KUNIT_EXPECT_EQ(test, ret, 0);

Should be KUNIT_ASSERT_EQ. If insert_extent() fails the test
continues and the next assertions run on uninitialized data. Same
for the base test.

No tag yet, fix the test data and I'll tag v4.


--- Thanks!
https://lore.kernel.org/all/[email protected]/

Reply via email to