From efa5fb5c3aa33298868281885ffd645e98cfbf75 Mon Sep 17 00:00:00 2001 From: Martin Janiczek Date: Wed, 11 Oct 2023 13:52:38 +0200 Subject: [PATCH] AK: Fix one-off error in BitmapView::find_first and find_one_anywhere The mentioned functions used m_size / 8 instead of size_in_bytes() (division with ceiling rounding mode), which resulted in an off-by-one error such that the functions didn't search in the last not-fully-8-bits byte. Using size_in_bytes() instead of m_size / 8 fixes this. --- AK/BitmapView.h | 6 +++--- Tests/AK/TestBitmap.cpp | 18 ++++++++++++++++++ 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/AK/BitmapView.h b/AK/BitmapView.h index 1bfb823567..98ea54b663 100644 --- a/AK/BitmapView.h +++ b/AK/BitmapView.h @@ -94,7 +94,7 @@ public: Optional find_one_anywhere(size_t hint = 0) const { VERIFY(hint < m_size); - u8 const* end = &m_data[m_size / 8]; + u8 const* end = &m_data[size_in_bytes()]; for (;;) { // We will use hint as what it is: a hint. Because we try to @@ -128,7 +128,7 @@ public: // We didn't find anything, check the remaining few bytes (if any) u8 byte = VALUE ? 0x00 : 0xff; size_t i = reinterpret_cast(ptr_large) - &m_data[0]; - size_t byte_count = m_size / 8; + size_t byte_count = size_in_bytes(); VERIFY(i <= byte_count); while (i < byte_count && m_data[i] == byte) i++; @@ -171,7 +171,7 @@ public: template Optional find_first() const { - size_t byte_count = m_size / 8; + size_t byte_count = size_in_bytes(); size_t i = 0; u8 byte = VALUE ? 0x00 : 0xff; diff --git a/Tests/AK/TestBitmap.cpp b/Tests/AK/TestBitmap.cpp index fedfd7f6c1..f7c14c94e6 100644 --- a/Tests/AK/TestBitmap.cpp +++ b/Tests/AK/TestBitmap.cpp @@ -276,3 +276,21 @@ TEST_CASE(byte_aligned_access) EXPECT_EQ(bitmap.count_in_range(4, 4, true), 1u); } } + +TEST_CASE(find_one_anywhere_edge_case) +{ + { + auto bitmap = MUST(Bitmap::create(1, false)); + bitmap.set(0, false); + EXPECT_EQ(bitmap.find_one_anywhere_unset(0).value(), 0UL); + } +} + +TEST_CASE(find_first_edge_case) +{ + { + auto bitmap = MUST(Bitmap::create(1, false)); + bitmap.set(0, false); + EXPECT_EQ(bitmap.find_first_unset().value(), 0UL); + } +}