From ebb1d9740e063baa5d22bcdca58d08354141aa1c Mon Sep 17 00:00:00 2001
From: Lenny Maiorani <lenny@colorado.edu>
Date: Sun, 16 May 2021 18:00:50 -0600
Subject: [PATCH] BitmapView: Disable mutations of the underlying Bitmap

Problem:
- `BitmapView` permits changing the underlying `Bitmap`. This violates
  the idea of a "view" since views are simply overlays which can
  themselves change but do not change the underlying data.

Solution:
- Migrate all non-`const` member functions to Bitmap.
---
 AK/Bitmap.h        | 91 +++++++++++++++++++++++++++++++++++++++++++---
 AK/BitmapView.h    | 84 ------------------------------------------
 Kernel/Heap/Heap.h |  4 +-
 3 files changed, 87 insertions(+), 92 deletions(-)

diff --git a/AK/Bitmap.h b/AK/Bitmap.h
index ce9ae52778..e58831379e 100644
--- a/AK/Bitmap.h
+++ b/AK/Bitmap.h
@@ -30,9 +30,10 @@ public:
         fill(default_value);
     }
 
-    Bitmap(u8* data, size_t size)
+    Bitmap(u8* data, size_t size, bool is_owning = false)
         : m_data(data)
         , m_size(size)
+        , m_is_owning(is_owning)
     {
     }
 
@@ -57,7 +58,9 @@ public:
 
     ~Bitmap()
     {
-        kfree(m_data);
+        if (m_is_owning) {
+            kfree(m_data);
+        }
         m_data = nullptr;
     }
 
@@ -108,11 +111,86 @@ public:
         }
     }
 
-    template<bool VALUE>
-    void set_range(size_t start, size_t len) { return view().set_range<VALUE, false>(start, len); }
-    void set_range(size_t start, size_t len, bool value) { return view().set_range(start, len, value); }
+    template<bool VALUE, bool verify_that_all_bits_flip = false>
+    void set_range(size_t start, size_t len)
+    {
+        VERIFY(start < m_size);
+        VERIFY(start + len <= m_size);
+        if (len == 0)
+            return;
 
-    void fill(bool value) { view().fill(value); }
+        static const u8 bitmask_first_byte[8] = { 0xFF, 0xFE, 0xFC, 0xF8, 0xF0, 0xE0, 0xC0, 0x80 };
+        static const u8 bitmask_last_byte[8] = { 0x0, 0x1, 0x3, 0x7, 0xF, 0x1F, 0x3F, 0x7F };
+
+        u8* first = &m_data[start / 8];
+        u8* last = &m_data[(start + len) / 8];
+        u8 byte_mask = bitmask_first_byte[start % 8];
+        if (first == last) {
+            byte_mask &= bitmask_last_byte[(start + len) % 8];
+            if constexpr (verify_that_all_bits_flip) {
+                if constexpr (VALUE) {
+                    VERIFY((*first & byte_mask) == 0);
+                } else {
+                    VERIFY((*first & byte_mask) == byte_mask);
+                }
+            }
+            if constexpr (VALUE)
+                *first |= byte_mask;
+            else
+                *first &= ~byte_mask;
+        } else {
+            if constexpr (verify_that_all_bits_flip) {
+                if constexpr (VALUE) {
+                    VERIFY((*first & byte_mask) == 0);
+                } else {
+                    VERIFY((*first & byte_mask) == byte_mask);
+                }
+            }
+            if constexpr (VALUE)
+                *first |= byte_mask;
+            else
+                *first &= ~byte_mask;
+            byte_mask = bitmask_last_byte[(start + len) % 8];
+            if constexpr (verify_that_all_bits_flip) {
+                if constexpr (VALUE) {
+                    VERIFY((*last & byte_mask) == 0);
+                } else {
+                    VERIFY((*last & byte_mask) == byte_mask);
+                }
+            }
+            if constexpr (VALUE)
+                *last |= byte_mask;
+            else
+                *last &= ~byte_mask;
+            if (++first < last) {
+                if constexpr (VALUE)
+                    __builtin_memset(first, 0xFF, last - first);
+                else
+                    __builtin_memset(first, 0x0, last - first);
+            }
+        }
+    }
+
+    void set_range(size_t start, size_t len, bool value)
+    {
+        if (value)
+            set_range<true, false>(start, len);
+        else
+            set_range<false, false>(start, len);
+    }
+
+    void set_range_and_verify_that_all_bits_flip(size_t start, size_t len, bool value)
+    {
+        if (value)
+            set_range<true, true>(start, len);
+        else
+            set_range<false, true>(start, len);
+    }
+
+    void fill(bool value)
+    {
+        __builtin_memset(m_data, value ? 0xff : 0x00, size_in_bytes());
+    }
 
     Optional<size_t> find_one_anywhere_set(size_t hint = 0) const { return view().find_one_anywhere<true>(hint); }
     Optional<size_t> find_one_anywhere_unset(size_t hint = 0) const { return view().find_one_anywhere<false>(hint); }
@@ -138,6 +216,7 @@ public:
 private:
     u8* m_data { nullptr };
     size_t m_size { 0 };
+    bool m_is_owning { true };
 };
 
 }
diff --git a/AK/BitmapView.h b/AK/BitmapView.h
index a2ae0342a7..993f557629 100644
--- a/AK/BitmapView.h
+++ b/AK/BitmapView.h
@@ -6,12 +6,10 @@
 
 #pragma once
 
-#include <AK/Noncopyable.h>
 #include <AK/Optional.h>
 #include <AK/Platform.h>
 #include <AK/StdLibExtras.h>
 #include <AK/Types.h>
-#include <AK/kmalloc.h>
 
 namespace AK {
 
@@ -95,90 +93,8 @@ public:
 
     bool is_null() const { return !m_data; }
 
-    u8* data() { return m_data; }
     const u8* data() const { return m_data; }
 
-    template<bool VALUE, bool verify_that_all_bits_flip>
-    void set_range(size_t start, size_t len)
-    {
-        VERIFY(start < m_size);
-        VERIFY(start + len <= m_size);
-        if (len == 0)
-            return;
-
-        static const u8 bitmask_first_byte[8] = { 0xFF, 0xFE, 0xFC, 0xF8, 0xF0, 0xE0, 0xC0, 0x80 };
-        static const u8 bitmask_last_byte[8] = { 0x0, 0x1, 0x3, 0x7, 0xF, 0x1F, 0x3F, 0x7F };
-
-        u8* first = &m_data[start / 8];
-        u8* last = &m_data[(start + len) / 8];
-        u8 byte_mask = bitmask_first_byte[start % 8];
-        if (first == last) {
-            byte_mask &= bitmask_last_byte[(start + len) % 8];
-            if constexpr (verify_that_all_bits_flip) {
-                if constexpr (VALUE) {
-                    VERIFY((*first & byte_mask) == 0);
-                } else {
-                    VERIFY((*first & byte_mask) == byte_mask);
-                }
-            }
-            if constexpr (VALUE)
-                *first |= byte_mask;
-            else
-                *first &= ~byte_mask;
-        } else {
-            if constexpr (verify_that_all_bits_flip) {
-                if constexpr (VALUE) {
-                    VERIFY((*first & byte_mask) == 0);
-                } else {
-                    VERIFY((*first & byte_mask) == byte_mask);
-                }
-            }
-            if constexpr (VALUE)
-                *first |= byte_mask;
-            else
-                *first &= ~byte_mask;
-            byte_mask = bitmask_last_byte[(start + len) % 8];
-            if constexpr (verify_that_all_bits_flip) {
-                if constexpr (VALUE) {
-                    VERIFY((*last & byte_mask) == 0);
-                } else {
-                    VERIFY((*last & byte_mask) == byte_mask);
-                }
-            }
-            if constexpr (VALUE)
-                *last |= byte_mask;
-            else
-                *last &= ~byte_mask;
-            if (++first < last) {
-                if constexpr (VALUE)
-                    __builtin_memset(first, 0xFF, last - first);
-                else
-                    __builtin_memset(first, 0x0, last - first);
-            }
-        }
-    }
-
-    void set_range(size_t start, size_t len, bool value)
-    {
-        if (value)
-            set_range<true, false>(start, len);
-        else
-            set_range<false, false>(start, len);
-    }
-
-    void set_range_and_verify_that_all_bits_flip(size_t start, size_t len, bool value)
-    {
-        if (value)
-            set_range<true, true>(start, len);
-        else
-            set_range<false, true>(start, len);
-    }
-
-    void fill(bool value)
-    {
-        __builtin_memset(m_data, value ? 0xff : 0x00, size_in_bytes());
-    }
-
     template<bool VALUE>
     Optional<size_t> find_one_anywhere(size_t hint = 0) const
     {
diff --git a/Kernel/Heap/Heap.h b/Kernel/Heap/Heap.h
index ef87e10ca7..ae4793a982 100644
--- a/Kernel/Heap/Heap.h
+++ b/Kernel/Heap/Heap.h
@@ -6,7 +6,7 @@
 
 #pragma once
 
-#include <AK/BitmapView.h>
+#include <AK/Bitmap.h>
 #include <AK/ScopeGuard.h>
 #include <AK/TemporaryChange.h>
 #include <AK/Vector.h>
@@ -153,7 +153,7 @@ private:
     size_t m_total_chunks { 0 };
     size_t m_allocated_chunks { 0 };
     u8* m_chunks { nullptr };
-    BitmapView m_bitmap;
+    Bitmap m_bitmap;
 };
 
 template<typename ExpandHeap>