AK+Kernel+LibELF: Remove the need for IteratorDecision::Continue

By constraining two implementations, the compiler will select the best fitting one. All this will require is duplicating the implementation and simplifying for the `void` case. This constraining also informs both the caller and compiler by passing the callback parameter types as part of the constraint (e.g.: `IterationFunction<int>`). Some `for_each` functions in LibELF only take functions which return `void`. This is a minimal correctness check, as it removes one way for a function to incompletely do something. There seems to be a possible idiom where inside a lambda, a `return;` is the same as `continue;` in a for-loop.
Author: https://github.com/Nicholas-Baron Commit: https://github.com/SerenityOS/serenity/commit/aa4d41fe2c4 Pull-request: https://github.com/SerenityOS/serenity/pull/6445 Reviewed-by: https://github.com/Hendiadyoin1 Reviewed-by: https://github.com/alimpfard Reviewed-by: https://github.com/linusg
2024-12-29 06:02:07 +03:00 · 2021-05-16 02:36:52 -07:00 · 2021-05-16 02:36:52 -07:00 · aa4d41fe2c · 2024-07-18 18:03:26 +09:00
commit aa4d41fe2c
parent bbaa463032
25 changed files with 311 additions and 127 deletions
--- a/AK/Concepts.h
+++ b/AK/Concepts.h
@ -6,6 +6,7 @@

 #pragma once

+#include <AK/IterationDecision.h>
 #include <AK/StdLibExtras.h>

 namespace AK::Concepts {
@ -27,6 +28,27 @@ concept Signed = IsSigned<T>;
 template<typename T>
 concept Unsigned = IsUnsigned<T>;

+template<typename T, typename U>
+concept SameAs = IsSame<T, U>;
+
+template<typename Func, typename... Args>
+concept VoidFunction = requires(Func func, Args... args)
+{
+    {
+        func(args...)
+    }
+    ->SameAs<void>;
+};
+
+template<typename Func, typename... Args>
+concept IteratorFunction = requires(Func func, Args... args)
+{
+    {
+        func(args...)
+    }
+    ->SameAs<IterationDecision>;
+};
+
 #endif

 }
@ -36,7 +58,9 @@ concept Unsigned = IsUnsigned<T>;
 using AK::Concepts::Arithmetic;
 using AK::Concepts::FloatingPoint;
 using AK::Concepts::Integral;
+using AK::Concepts::IteratorFunction;
 using AK::Concepts::Signed;
 using AK::Concepts::Unsigned;
+using AK::Concepts::VoidFunction;

 #endif
--- a/AK/InlineLinkedList.h
+++ b/AK/InlineLinkedList.h
@ -7,6 +7,7 @@
 #pragma once

 #include <AK/Assertions.h>
+#include <AK/Concepts.h>
 #include <AK/Types.h>

 namespace AK {
@ -112,7 +113,7 @@ public:
        return false;
    }

-    template<typename F>
+    template<IteratorFunction<T&> F>
    IterationDecision for_each(F func) const
    {
        for (T* node = m_head; node; node = node->next()) {
@ -123,6 +124,13 @@ public:
        return IterationDecision::Continue;
    }

+    template<VoidFunction<T&> F>
+    void for_each(F func) const
+    {
+        for (T* node = m_head; node; node = node->next())
+            func(*node);
+    }
+
    using Iterator = InlineLinkedListIterator<T>;
    friend Iterator;
    Iterator begin() { return Iterator(m_head); }
--- a/Kernel/Arch/i386/CPU.cpp
+++ b/Kernel/Arch/i386/CPU.cpp
@ -2090,12 +2090,11 @@ void Processor::smp_broadcast_message(ProcessorMessage& msg)
    VERIFY(msg.refs > 0);
    bool need_broadcast = false;
    for_each(
-        [&](Processor& proc) -> IterationDecision {
+        [&](Processor& proc) {
            if (&proc != &cur_proc) {
                if (proc.smp_queue_message(msg))
                    need_broadcast = true;
            }
-            return IterationDecision::Continue;
        });

    // Now trigger an IPI on all other APs (unless all targets already had messages queued)
@ -2215,9 +2214,8 @@ void Processor::smp_broadcast_halt()
    // We don't want to use a message, because this could have been triggered
    // by being out of memory and we might not be able to get a message
    for_each(
-        [&](Processor& proc) -> IterationDecision {
+        [&](Processor& proc) {
            proc.m_halt_requested.store(true, AK::MemoryOrder::memory_order_release);
-            return IterationDecision::Continue;
        });

    // Now trigger an IPI on all other APs
--- a/Kernel/Arch/x86/CPU.h
+++ b/Kernel/Arch/x86/CPU.h
@ -8,6 +8,7 @@

 #include <AK/Atomic.h>
 #include <AK/Badge.h>
+#include <AK/Concepts.h>
 #include <AK/Noncopyable.h>
 #include <AK/Vector.h>

@ -733,7 +734,7 @@ public:

    static size_t processor_count() { return processors().size(); }

-    template<typename Callback>
+    template<IteratorFunction<Processor&> Callback>
    static inline IterationDecision for_each(Callback callback)
    {
        auto& procs = processors();
@ -745,6 +746,16 @@ public:
        return IterationDecision::Continue;
    }

+    template<VoidFunction<Processor&> Callback>
+    static inline IterationDecision for_each(Callback callback)
+    {
+        auto& procs = processors();
+        size_t count = procs.size();
+        for (size_t i = 0; i < count; i++)
+            callback(*procs[i]);
+        return IterationDecision::Continue;
+    }
+
    ALWAYS_INLINE u8 physical_address_bit_width() const { return m_physical_address_bit_width; }

    ALWAYS_INLINE ProcessorInfo& info() { return *m_info; }
--- a/Kernel/FileSystem/ProcFS.cpp
+++ b/Kernel/FileSystem/ProcFS.cpp
@ -688,7 +688,7 @@ static bool procfs$cpuinfo(InodeIdentifier, KBufferBuilder& builder)
 {
    JsonArraySerializer array { builder };
    Processor::for_each(
-        [&](Processor& proc) -> IterationDecision {
+        [&](Processor& proc) {
            auto& info = proc.info();
            auto obj = array.add_object();
            JsonArray features;
@ -702,7 +702,6 @@ static bool procfs$cpuinfo(InodeIdentifier, KBufferBuilder& builder)
            obj.add("stepping", info.stepping());
            obj.add("type", info.type());
            obj.add("brandstr", info.brandstr());
-            return IterationDecision::Continue;
        });
    array.finish();
    return true;
@ -826,7 +825,6 @@ static bool procfs$all(InodeIdentifier, KBufferBuilder& builder)
            thread_object.add("unix_socket_write_bytes", thread.unix_socket_write_bytes());
            thread_object.add("ipv4_socket_read_bytes", thread.ipv4_socket_read_bytes());
            thread_object.add("ipv4_socket_write_bytes", thread.ipv4_socket_write_bytes());
-            return IterationDecision::Continue;
        });
    };

@ -1328,10 +1326,9 @@ KResult ProcFSInode::traverse_as_directory(Function<bool(const FS::DirectoryEntr
        auto process = Process::from_pid(pid);
        if (!process)
            return ENOENT;
-        process->for_each_thread([&](const Thread& thread) -> IterationDecision {
+        process->for_each_thread([&](const Thread& thread) {
            int tid = thread.tid().value();
            callback({ String::number(tid), to_identifier_with_stack(fsid(), tid), 0 });
-            return IterationDecision::Continue;
        });
    } break;

--- a/Kernel/PerformanceEventBuffer.cpp
+++ b/Kernel/PerformanceEventBuffer.cpp
@ -230,7 +230,6 @@ void PerformanceEventBuffer::add_process(const Process& process, ProcessEventTyp
    process.for_each_thread([&](auto& thread) {
        [[maybe_unused]] auto rc = append_with_eip_and_ebp(process.pid(), thread.tid().value(),
            0, 0, PERF_EVENT_THREAD_CREATE, 0, 0, 0, nullptr);
-        return IterationDecision::Continue;
    });

    for (auto& region : process.space().regions()) {
--- a/Kernel/Process.cpp
+++ b/Kernel/Process.cpp
@ -103,15 +103,16 @@ void Process::kill_threads_except_self()

    auto current_thread = Thread::current();
    for_each_thread([&](Thread& thread) {
-        if (&thread == current_thread
-            || thread.state() == Thread::State::Dead
-            || thread.state() == Thread::State::Dying)
-            return IterationDecision::Continue;
+        if (&thread == current_thread)
+            return;
+
+        if (auto state = thread.state(); state == Thread::State::Dead
+            || state == Thread::State::Dying)
+            return;

        // We need to detach this thread in case it hasn't been joined
        thread.detach();
        thread.set_should_die();
-        return IterationDecision::Continue;
    });

    big_lock().clear_waiters();
@ -123,7 +124,6 @@ void Process::kill_all_threads()
        // We need to detach this thread in case it hasn't been joined
        thread.detach();
        thread.set_should_die();
-        return IterationDecision::Continue;
    });
 }

@ -562,7 +562,6 @@ void Process::die()

    for_each_thread([&](auto& thread) {
        m_threads_for_coredump.append(thread);
-        return IterationDecision::Continue;
    });

    {
--- a/Kernel/Process.h
+++ b/Kernel/Process.h
@ -7,6 +7,7 @@
 #pragma once

 #include <AK/Checked.h>
+#include <AK/Concepts.h>
 #include <AK/HashMap.h>
 #include <AK/InlineLinkedList.h>
 #include <AK/NonnullOwnPtrVector.h>
@ -224,16 +225,30 @@ public:
    RefPtr<FileDescription> file_description(int fd) const;
    int fd_flags(int fd) const;

-    template<typename Callback>
+    // Breakable iteration functions
+    template<IteratorFunction<Process&> Callback>
    static void for_each(Callback);
-    template<typename Callback>
+    template<IteratorFunction<Process&> Callback>
    static void for_each_in_pgrp(ProcessGroupID, Callback);
-    template<typename Callback>
+    template<IteratorFunction<Process&> Callback>
    void for_each_child(Callback);

-    template<typename Callback>
+    template<IteratorFunction<Thread&> Callback>
    IterationDecision for_each_thread(Callback);
-    template<typename Callback>
+    template<IteratorFunction<Thread&> Callback>
+    IterationDecision for_each_thread(Callback callback) const;
+
+    // Non-breakable iteration functions
+    template<VoidFunction<Process&> Callback>
+    static void for_each(Callback);
+    template<VoidFunction<Process&> Callback>
+    static void for_each_in_pgrp(ProcessGroupID, Callback);
+    template<VoidFunction<Process&> Callback>
+    void for_each_child(Callback);
+
+    template<VoidFunction<Thread&> Callback>
+    IterationDecision for_each_thread(Callback);
+    template<VoidFunction<Thread&> Callback>
    IterationDecision for_each_thread(Callback callback) const;

    void die();
@ -628,7 +643,7 @@ private:
 extern InlineLinkedList<Process>* g_processes;
 extern RecursiveSpinLock g_processes_lock;

-template<typename Callback>
+template<IteratorFunction<Process&> Callback>
 inline void Process::for_each(Callback callback)
 {
    VERIFY_INTERRUPTS_DISABLED();
@ -641,7 +656,7 @@ inline void Process::for_each(Callback callback)
    }
 }

-template<typename Callback>
+template<IteratorFunction<Process&> Callback>
 inline void Process::for_each_child(Callback callback)
 {
    VERIFY_INTERRUPTS_DISABLED();
@ -657,7 +672,7 @@ inline void Process::for_each_child(Callback callback)
    }
 }

-template<typename Callback>
+template<IteratorFunction<Thread&> Callback>
 inline IterationDecision Process::for_each_thread(Callback callback) const
 {
    ScopedSpinLock thread_list_lock(m_thread_list_lock);
@ -669,7 +684,7 @@ inline IterationDecision Process::for_each_thread(Callback callback) const
    return IterationDecision::Continue;
 }

-template<typename Callback>
+template<IteratorFunction<Thread&> Callback>
 inline IterationDecision Process::for_each_thread(Callback callback)
 {
    ScopedSpinLock thread_list_lock(m_thread_list_lock);
@ -681,7 +696,7 @@ inline IterationDecision Process::for_each_thread(Callback callback)
    return IterationDecision::Continue;
 }

-template<typename Callback>
+template<IteratorFunction<Process&> Callback>
 inline void Process::for_each_in_pgrp(ProcessGroupID pgid, Callback callback)
 {
    VERIFY_INTERRUPTS_DISABLED();
@ -696,6 +711,51 @@ inline void Process::for_each_in_pgrp(ProcessGroupID pgid, Callback callback)
    }
 }

+template<VoidFunction<Process&> Callback>
+inline void Process::for_each(Callback callback)
+{
+    return for_each([&](auto& item) {
+        callback(item);
+        return IterationDecision::Continue;
+    });
+}
+
+template<VoidFunction<Process&> Callback>
+inline void Process::for_each_child(Callback callback)
+{
+    return for_each_child([&](auto& item) {
+        callback(item);
+        return IterationDecision::Continue;
+    });
+}
+
+template<VoidFunction<Thread&> Callback>
+inline IterationDecision Process::for_each_thread(Callback callback) const
+{
+    ScopedSpinLock thread_list_lock(m_thread_list_lock);
+    for (auto& thread : m_thread_list)
+        callback(thread);
+    return IterationDecision::Continue;
+}
+
+template<VoidFunction<Thread&> Callback>
+inline IterationDecision Process::for_each_thread(Callback callback)
+{
+    ScopedSpinLock thread_list_lock(m_thread_list_lock);
+    for (auto& thread : m_thread_list)
+        callback(thread);
+    return IterationDecision::Continue;
+}
+
+template<VoidFunction<Process&> Callback>
+inline void Process::for_each_in_pgrp(ProcessGroupID pgid, Callback callback)
+{
+    return for_each_in_pgrp(pgid, [&](auto& item) {
+        callback(item);
+        return IterationDecision::Continue;
+    });
+}
+
 inline bool InodeMetadata::may_read(const Process& process) const
 {
    return may_read(process.euid(), process.egid(), process.extra_gids());
--- a/Kernel/Scheduler.cpp
+++ b/Kernel/Scheduler.cpp
@ -584,7 +584,7 @@ void dump_thread_list()
        return thread.get_register_dump_from_stack().eip;
    };

-    Thread::for_each([&](Thread& thread) -> IterationDecision {
+    Thread::for_each([&](Thread& thread) {
        switch (thread.state()) {
        case Thread::Dying:
            dbgln("  {:14} {:30} @ {:04x}:{:08x} Finalizable: {}, (nsched: {})",
@ -605,8 +605,6 @@ void dump_thread_list()
                thread.times_scheduled());
            break;
        }
-
-        return IterationDecision::Continue;
    });
 }

--- a/Kernel/Syscalls/execve.cpp
+++ b/Kernel/Syscalls/execve.cpp
@ -172,7 +172,7 @@ static KResultOr<RequiredLoadRange> get_required_load_range(FileDescription& pro
    RequiredLoadRange range {};
    elf_image.for_each_program_header([&range](const auto& pheader) {
        if (pheader.type() != PT_LOAD)
-            return IterationDecision::Continue;
+            return;

        auto region_start = (FlatPtr)pheader.vaddr().as_ptr();
        auto region_end = region_start + pheader.size_in_memory();
@ -180,7 +180,6 @@ static KResultOr<RequiredLoadRange> get_required_load_range(FileDescription& pro
            range.start = region_start;
        if (range.end == 0 || region_end > range.end)
            range.end = region_end;
-        return IterationDecision::Continue;
    });

    VERIFY(range.end > range.start);
--- a/Kernel/Syscalls/kill.cpp
+++ b/Kernel/Syscalls/kill.cpp
@ -47,8 +47,6 @@ KResult Process::do_killpg(ProcessGroupID pgrp, int signal)
            any_succeeded = true;
        else
            error = res;
-
-        return IterationDecision::Continue;
    });

    if (group_was_empty)
--- a/Kernel/Syscalls/module.cpp
+++ b/Kernel/Syscalls/module.cpp
@ -51,18 +51,18 @@ KResultOr<int> Process::sys$module_load(Userspace<const char*> user_path, size_t

    elf_image->for_each_section_of_type(SHT_PROGBITS, [&](const ELF::Image::Section& section) {
        if (!section.size())
-            return IterationDecision::Continue;
+            return;
        auto section_storage = KBuffer::copy(section.raw_data(), section.size(), Region::Access::Read | Region::Access::Write | Region::Access::Execute);
        section_storage_by_name.set(section.name(), section_storage.data());
        module->sections.append(move(section_storage));
-        return IterationDecision::Continue;
    });

    bool missing_symbols = false;

    elf_image->for_each_section_of_type(SHT_PROGBITS, [&](const ELF::Image::Section& section) {
        if (!section.size())
-            return IterationDecision::Continue;
+            return;
+
        auto* section_storage = section_storage_by_name.get(section.name()).value_or(nullptr);
        VERIFY(section_storage);
        auto relocations = section.relocations();
@ -103,10 +103,7 @@ KResultOr<int> Process::sys$module_load(Userspace<const char*> user_path, size_t
                }
                break;
            }
-            return IterationDecision::Continue;
        });
-
-        return IterationDecision::Continue;
    });

    if (missing_symbols)
@ -129,7 +126,6 @@ KResultOr<int> Process::sys$module_load(Userspace<const char*> user_path, size_t
            if (storage)
                module->name = String((const char*)(storage + symbol.value()));
        }
-        return IterationDecision::Continue;
    });

    if (!module->module_init)
--- a/Kernel/TTY/TTY.cpp
+++ b/Kernel/TTY/TTY.cpp
@ -286,7 +286,6 @@ void TTY::generate_signal(int signal)
        dbgln_if(TTY_DEBUG, "{}: Send signal {} to {}", tty_name(), signal, process);
        // FIXME: Should this error be propagated somehow?
        [[maybe_unused]] auto rc = process.send_signal(signal, nullptr);
-        return IterationDecision::Continue;
    });
 }

--- a/Kernel/Thread.cpp
+++ b/Kernel/Thread.cpp
@ -404,7 +404,6 @@ void Thread::finalize_dying_threads()
        for_each_in_state(Thread::State::Dying, [&](Thread& thread) {
            if (thread.is_finalizable())
                dying_threads.append(&thread);
-            return IterationDecision::Continue;
        });
    }
    for (auto* thread : dying_threads) {
@ -740,7 +739,6 @@ DispatchSignalResult Thread::dispatch_signal(u8 signal)
            process.set_dump_core(true);
            process.for_each_thread([](auto& thread) {
                thread.set_dump_backtrace_on_finalization();
-                return IterationDecision::Continue;
            });
            [[fallthrough]];
        case DefaultSignalAction::Terminate:
@ -891,11 +889,12 @@ void Thread::set_state(State new_state, u8 stop_signal)
        auto& process = this->process();
        if (process.set_stopped(false) == true) {
            process.for_each_thread([&](auto& thread) {
-                if (&thread == this || !thread.is_stopped())
-                    return IterationDecision::Continue;
+                if (&thread == this)
+                    return;
+                if (!thread.is_stopped())
+                    return;
                dbgln_if(THREAD_DEBUG, "Resuming peer thread {}", thread);
                thread.resume_from_stopped();
-                return IterationDecision::Continue;
            });
            process.unblock_waiters(Thread::WaitBlocker::UnblockFlags::Continued);
            // Tell the parent process (if any) about this change.
@ -914,11 +913,12 @@ void Thread::set_state(State new_state, u8 stop_signal)
        auto& process = this->process();
        if (process.set_stopped(true) == false) {
            process.for_each_thread([&](auto& thread) {
-                if (&thread == this || thread.is_stopped())
-                    return IterationDecision::Continue;
+                if (&thread == this)
+                    return;
+                if (thread.is_stopped())
+                    return;
                dbgln_if(THREAD_DEBUG, "Stopping peer thread {}", thread);
                thread.set_state(Stopped, stop_signal);
-                return IterationDecision::Continue;
            });
            process.unblock_waiters(Thread::WaitBlocker::UnblockFlags::Stopped, stop_signal);
            // Tell the parent process (if any) about this change.
--- a/Kernel/Thread.h
+++ b/Kernel/Thread.h
@ -6,6 +6,7 @@

 #pragma once

+#include <AK/Concepts.h>
 #include <AK/EnumBits.h>
 #include <AK/Function.h>
 #include <AK/HashMap.h>
@ -1044,9 +1045,14 @@ public:

    RefPtr<Thread> clone(Process&);

-    template<typename Callback>
+    template<IteratorFunction<Thread&> Callback>
    static IterationDecision for_each_in_state(State, Callback);
-    template<typename Callback>
+    template<IteratorFunction<Thread&> Callback>
+    static IterationDecision for_each(Callback);
+
+    template<VoidFunction<Thread&> Callback>
+    static IterationDecision for_each_in_state(State, Callback);
+    template<VoidFunction<Thread&> Callback>
    static IterationDecision for_each(Callback);

    static constexpr u32 default_kernel_stack_size = 65536;
@ -1260,7 +1266,7 @@ private:

 AK_ENUM_BITWISE_OPERATORS(Thread::FileBlocker::BlockFlags);

-template<typename Callback>
+template<IteratorFunction<Thread&> Callback>
 inline IterationDecision Thread::for_each(Callback callback)
 {
    ScopedSpinLock lock(g_tid_map_lock);
@ -1272,7 +1278,7 @@ inline IterationDecision Thread::for_each(Callback callback)
    return IterationDecision::Continue;
 }

-template<typename Callback>
+template<IteratorFunction<Thread&> Callback>
 inline IterationDecision Thread::for_each_in_state(State state, Callback callback)
 {
    ScopedSpinLock lock(g_tid_map_lock);
@ -1287,6 +1293,24 @@ inline IterationDecision Thread::for_each_in_state(State state, Callback callbac
    return IterationDecision::Continue;
 }

+template<VoidFunction<Thread&> Callback>
+inline IterationDecision Thread::for_each(Callback callback)
+{
+    ScopedSpinLock lock(g_tid_map_lock);
+    for (auto& it : *g_tid_map)
+        callback(*it.value);
+    return IterationDecision::Continue;
+}
+
+template<VoidFunction<Thread&> Callback>
+inline IterationDecision Thread::for_each_in_state(State state, Callback callback)
+{
+    return for_each_in_state(state, [&](auto& thread) {
+        callback(thread);
+        return IterationDecision::Continue;
+    });
+}
+
 }

 template<>
--- a/Kernel/VM/AnonymousVMObject.cpp
+++ b/Kernel/VM/AnonymousVMObject.cpp
@ -23,13 +23,11 @@ RefPtr<VMObject> AnonymousVMObject::clone()
    // so that the parent is still guaranteed to be able to have all
    // non-volatile memory available.
    size_t need_cow_pages = 0;
-    {
-        // We definitely need to commit non-volatile areas
-        for_each_nonvolatile_range([&](const VolatilePageRange& nonvolatile_range) {
-            need_cow_pages += nonvolatile_range.count;
-            return IterationDecision::Continue;
-        });
-    }
+
+    // We definitely need to commit non-volatile areas
+    for_each_nonvolatile_range([&](const VolatilePageRange& nonvolatile_range) {
+        need_cow_pages += nonvolatile_range.count;
+    });

    dbgln_if(COMMIT_DEBUG, "Cloning {:p}, need {} committed cow pages", this, need_cow_pages);

@ -220,7 +218,6 @@ int AnonymousVMObject::purge_impl()
                }
            });
        }
-        return IterationDecision::Continue;
    });
    return purged_page_count;
 }
@ -284,7 +281,6 @@ void AnonymousVMObject::update_volatile_cache()
    m_volatile_ranges_cache.clear();
    for_each_nonvolatile_range([&](const VolatilePageRange& range) {
        m_volatile_ranges_cache.add_unchecked(range);
-        return IterationDecision::Continue;
    });

    m_volatile_ranges_cache_dirty = false;
--- a/Kernel/VM/AnonymousVMObject.h
+++ b/Kernel/VM/AnonymousVMObject.h
@ -40,7 +40,7 @@ public:

    bool is_any_volatile() const;

-    template<typename F>
+    template<IteratorFunction<const VolatilePageRange&> F>
    IterationDecision for_each_volatile_range(F f) const
    {
        VERIFY(m_lock.is_locked());
@ -78,24 +78,42 @@ public:
        return IterationDecision::Continue;
    }

-    template<typename F>
+    template<IteratorFunction<const VolatilePageRange&> F>
    IterationDecision for_each_nonvolatile_range(F f) const
    {
        size_t base = 0;
        for_each_volatile_range([&](const VolatilePageRange& volatile_range) {
            if (volatile_range.base == base)
                return IterationDecision::Continue;
-            IterationDecision decision = f({ base, volatile_range.base - base });
+            IterationDecision decision = f(VolatilePageRange { base, volatile_range.base - base });
            if (decision != IterationDecision::Continue)
                return decision;
            base = volatile_range.base + volatile_range.count;
            return IterationDecision::Continue;
        });
        if (base < page_count())
-            return f({ base, page_count() - base });
+            return f(VolatilePageRange { base, page_count() - base });
        return IterationDecision::Continue;
    }

+    template<VoidFunction<const VolatilePageRange&> F>
+    IterationDecision for_each_volatile_range(F f) const
+    {
+        return for_each_volatile_range([&](auto& range) {
+            f(range);
+            return IterationDecision::Continue;
+        });
+    }
+
+    template<VoidFunction<const VolatilePageRange&> F>
+    IterationDecision for_each_nonvolatile_range(F f) const
+    {
+        return for_each_nonvolatile_range([&](auto range) {
+            f(move(range));
+            return IterationDecision::Continue;
+        });
+    }
+
 private:
    explicit AnonymousVMObject(size_t, AllocationStrategy);
    explicit AnonymousVMObject(PhysicalAddress, size_t);
--- a/Kernel/VM/MemoryManager.h
+++ b/Kernel/VM/MemoryManager.h
@ -6,6 +6,7 @@

 #pragma once

+#include <AK/Concepts.h>
 #include <AK/HashTable.h>
 #include <AK/NonnullRefPtrVector.h>
 #include <AK/String.h>
@ -157,7 +158,7 @@ public:
    unsigned super_physical_pages() const { return m_super_physical_pages; }
    unsigned super_physical_pages_used() const { return m_super_physical_pages_used; }

-    template<typename Callback>
+    template<IteratorFunction<VMObject&> Callback>
    static void for_each_vmobject(Callback callback)
    {
        for (auto& vmobject : MM.m_vmobjects) {
@ -166,6 +167,13 @@ public:
        }
    }

+    template<VoidFunction<VMObject&> Callback>
+    static void for_each_vmobject(Callback callback)
+    {
+        for (auto& vmobject : MM.m_vmobjects)
+            callback(vmobject);
+    }
+
    static Region* find_region_from_vaddr(Space&, VirtualAddress);
    static Region* find_user_region_from_vaddr(Space&, VirtualAddress);

--- a/Userland/Libraries/LibELF/DynamicLinker.cpp
+++ b/Userland/Libraries/LibELF/DynamicLinker.cpp
@ -125,9 +125,8 @@ static Vector<String> get_dependencies(const String& name)

    lib->for_each_needed_library([&dependencies, &name](auto needed_name) {
        if (name == needed_name)
-            return IterationDecision::Continue;
+            return;
        dependencies.append(needed_name);
-        return IterationDecision::Continue;
    });
    return dependencies;
 }
--- a/Userland/Libraries/LibELF/DynamicLoader.cpp
+++ b/Userland/Libraries/LibELF/DynamicLoader.cpp
@ -93,7 +93,6 @@ const DynamicObject& DynamicLoader::dynamic_object() const
            if (program_header.type() == PT_DYNAMIC) {
                dynamic_section_address = VirtualAddress(program_header.raw_data());
            }
-            return IterationDecision::Continue;
        });
        VERIFY(!dynamic_section_address.is_null());

@ -109,7 +108,6 @@ size_t DynamicLoader::calculate_tls_size() const
        if (program_header.type() == PT_TLS) {
            tls_size = program_header.size_in_memory();
        }
-        return IterationDecision::Continue;
    });
    return tls_size;
 }
@ -194,7 +192,6 @@ void DynamicLoader::do_main_relocations()
        case RelocationResult::Success:
            break;
        }
-        return IterationDecision::Continue;
    };
    m_dynamic_object->relocation_section().for_each_relocation(do_single_relocation);
    m_dynamic_object->plt_relocation_section().for_each_relocation(do_single_relocation);
@ -273,7 +270,6 @@ void DynamicLoader::load_program_headers()
            VERIFY(!relro_region.has_value());
            relro_region = region;
        }
-        return IterationDecision::Continue;
    });

    VERIFY(!text_regions.is_empty() || !data_regions.is_empty());
--- a/Userland/Libraries/LibELF/DynamicObject.cpp
+++ b/Userland/Libraries/LibELF/DynamicObject.cpp
@ -49,7 +49,6 @@ void DynamicObject::dump() const
        String name_field = String::formatted("({})", name_for_dtag(entry.tag()));
        builder.appendff("{:#08x} {:17} {:#08x}\n", entry.tag(), name_field, entry.val());
        num_dynamic_sections++;
-        return IterationDecision::Continue;
    });

    if (m_has_soname)
@ -171,7 +170,6 @@ void DynamicObject::parse()
            VERIFY_NOT_REACHED(); // FIXME: Maybe just break out here and return false?
            break;
        }
-        return IterationDecision::Continue;
    });

    if (!m_size_of_relocation_entry) {
--- a/Userland/Libraries/LibELF/DynamicObject.h
+++ b/Userland/Libraries/LibELF/DynamicObject.h
@ -8,6 +8,7 @@
 #pragma once

 #include <AK/Assertions.h>
+#include <AK/Concepts.h>
 #include <AK/RefCounted.h>
 #include <AK/String.h>
 #include <Kernel/VirtualAddress.h>
@ -125,8 +126,11 @@ public:
        unsigned relocation_count() const { return entry_count(); }
        Relocation relocation(unsigned index) const;
        Relocation relocation_at_offset(unsigned offset) const;
-        template<typename F>
+
+        template<IteratorFunction<DynamicObject::Relocation&> F>
        void for_each_relocation(F) const;
+        template<VoidFunction<DynamicObject::Relocation&> F>
+        void for_each_relocation(F func) const;
    };

    class Relocation {
@ -251,16 +255,18 @@ public:
    ElfW(Half) program_header_count() const;
    const ElfW(Phdr) * program_headers() const;

-    template<typename F>
+    template<VoidFunction<StringView> F>
    void for_each_needed_library(F) const;

-    template<typename F>
+    template<VoidFunction<InitializationFunction&> F>
    void for_each_initialization_array_function(F f) const;

-    template<typename F>
+    template<IteratorFunction<DynamicEntry&> F>
    void for_each_dynamic_entry(F) const;
+    template<VoidFunction<DynamicEntry&> F>
+    void for_each_dynamic_entry(F func) const;

-    template<typename F>
+    template<VoidFunction<Symbol&> F>
    void for_each_symbol(F) const;

    struct SymbolLookupResult {
@ -341,7 +347,7 @@ private:
    // End Section information from DT_* entries
 };

-template<typename F>
+template<IteratorFunction<DynamicObject::Relocation&> F>
 inline void DynamicObject::RelocationSection::for_each_relocation(F func) const
 {
    for (unsigned i = 0; i < relocation_count(); ++i) {
@ -353,16 +359,24 @@ inline void DynamicObject::RelocationSection::for_each_relocation(F func) const
    }
 }

-template<typename F>
+template<VoidFunction<DynamicObject::Relocation&> F>
+inline void DynamicObject::RelocationSection::for_each_relocation(F func) const
+{
+    for_each_relocation([&](auto& reloc) {
+        func(reloc);
+        return IterationDecision::Continue;
+    });
+}
+
+template<VoidFunction<DynamicObject::Symbol&> F>
 inline void DynamicObject::for_each_symbol(F func) const
 {
    for (unsigned i = 0; i < symbol_count(); ++i) {
-        if (func(symbol(i)) == IterationDecision::Break)
-            break;
+        func(symbol(i));
    }
 }

-template<typename F>
+template<IteratorFunction<DynamicObject::DynamicEntry&> F>
 inline void DynamicObject::for_each_dynamic_entry(F func) const
 {
    auto* dyns = reinterpret_cast<const ElfW(Dyn)*>(m_dynamic_address.as_ptr());
@ -374,21 +388,29 @@ inline void DynamicObject::for_each_dynamic_entry(F func) const
            break;
    }
 }
-template<typename F>
-inline void DynamicObject::for_each_needed_library(F func) const
+
+template<VoidFunction<DynamicObject::DynamicEntry&> F>
+inline void DynamicObject::for_each_dynamic_entry(F func) const
 {
-    for_each_dynamic_entry([func, this](auto entry) {
-        if (entry.tag() != DT_NEEDED)
-            return IterationDecision::Continue;
-        ElfW(Word) offset = entry.val();
-        StringView name { (const char*)(m_base_address.offset(m_string_table_offset).offset(offset)).as_ptr() };
-        if (func(StringView(name)) == IterationDecision::Break)
-            return IterationDecision::Break;
+    for_each_dynamic_entry([&](auto& dyn) {
+        func(dyn);
        return IterationDecision::Continue;
    });
 }

-template<typename F>
+template<VoidFunction<StringView> F>
+inline void DynamicObject::for_each_needed_library(F func) const
+{
+    for_each_dynamic_entry([func, this](auto entry) {
+        if (entry.tag() != DT_NEEDED)
+            return;
+        ElfW(Word) offset = entry.val();
+        StringView name { (const char*)(m_base_address.offset(m_string_table_offset).offset(offset)).as_ptr() };
+        func(name);
+    });
+}
+
+template<VoidFunction<DynamicObject::InitializationFunction&> F>
 void DynamicObject::for_each_initialization_array_function(F f) const
 {
    if (!has_init_array_section())
--- a/Userland/Libraries/LibELF/Image.cpp
+++ b/Userland/Libraries/LibELF/Image.cpp
@ -97,7 +97,6 @@ void Image::dump() const
        dbgln("      offset: {:x}", program_header.offset());
        dbgln("       flags: {:x}", program_header.flags());
        dbgln("    }}");
-        return IterationDecision::Continue;
    });

    for (unsigned i = 0; i < header().e_shnum; ++i) {
@ -344,7 +343,6 @@ NEVER_INLINE void Image::sort_symbols() const
    m_sorted_symbols.ensure_capacity(symbol_count());
    for_each_symbol([this](const auto& symbol) {
        m_sorted_symbols.append({ symbol.value(), symbol.name(), {}, symbol });
-        return IterationDecision::Continue;
    });
    quick_sort(m_sorted_symbols, [](auto& a, auto& b) {
        return a.address < b.address;
--- a/Userland/Libraries/LibELF/Image.h
+++ b/Userland/Libraries/LibELF/Image.h
@ -6,6 +6,7 @@

 #pragma once

+#include <AK/Concepts.h>
 #include <AK/String.h>
 #include <AK/Vector.h>
 #include <Kernel/VirtualAddress.h>
@ -134,7 +135,8 @@ public:
        }
        unsigned relocation_count() const { return entry_count(); }
        Relocation relocation(unsigned index) const;
-        template<typename F>
+
+        template<VoidFunction<Image::Relocation&> F>
        void for_each_relocation(F) const;
    };

@ -167,13 +169,24 @@ public:
    ProgramHeader program_header(unsigned) const;
    FlatPtr program_header_table_offset() const;

-    template<typename F>
+    template<IteratorFunction<Image::Section> F>
    void for_each_section(F) const;
-    template<typename F>
+    template<VoidFunction<Section> F>
+    void for_each_section(F) const;
+
+    template<IteratorFunction<Section&> F>
    void for_each_section_of_type(unsigned, F) const;
-    template<typename F>
+    template<VoidFunction<Section&> F>
+    void for_each_section_of_type(unsigned, F) const;
+
+    template<IteratorFunction<Symbol> F>
    void for_each_symbol(F) const;
-    template<typename F>
+    template<VoidFunction<Symbol> F>
+    void for_each_symbol(F) const;
+
+    template<IteratorFunction<ProgramHeader> F>
+    void for_each_program_header(F func) const;
+    template<VoidFunction<ProgramHeader> F>
    void for_each_program_header(F) const;

    Optional<Section> lookup_section(String const& name) const;
@ -222,15 +235,26 @@ private:
    mutable Vector<SortedSymbol> m_sorted_symbols;
 };

-template<typename F>
+template<IteratorFunction<Image::Section> F>
 inline void Image::for_each_section(F func) const
 {
    auto section_count = this->section_count();
-    for (unsigned i = 0; i < section_count; ++i)
-        func(section(i));
+    for (unsigned i = 0; i < section_count; ++i) {
+        if (func(section(i)) == IterationDecision::Break)
+            break;
+    }
 }

-template<typename F>
+template<VoidFunction<Image::Section> F>
+inline void Image::for_each_section(F func) const
+{
+    for_each_section([&](auto section) {
+        func(move(section));
+        return IterationDecision::Continue;
+    });
+}
+
+template<IteratorFunction<Image::Section&> F>
 inline void Image::for_each_section_of_type(unsigned type, F func) const
 {
    auto section_count = this->section_count();
@ -243,17 +267,25 @@ inline void Image::for_each_section_of_type(unsigned type, F func) const
    }
 }

-template<typename F>
+template<VoidFunction<Image::Section&> F>
+inline void Image::for_each_section_of_type(unsigned type, F func) const
+{
+    for_each_section_of_type(type, [&](auto& section) {
+        func(section);
+        return IterationDecision::Continue;
+    });
+}
+
+template<VoidFunction<Image::Relocation&> F>
 inline void Image::RelocationSection::for_each_relocation(F func) const
 {
    auto relocation_count = this->relocation_count();
    for (unsigned i = 0; i < relocation_count; ++i) {
-        if (func(relocation(i)) == IterationDecision::Break)
-            break;
+        func(relocation(i));
    }
 }

-template<typename F>
+template<IteratorFunction<Image::Symbol> F>
 inline void Image::for_each_symbol(F func) const
 {
    auto symbol_count = this->symbol_count();
@ -263,14 +295,32 @@ inline void Image::for_each_symbol(F func) const
    }
 }

-template<typename F>
+template<VoidFunction<Image::Symbol> F>
+inline void Image::for_each_symbol(F func) const
+{
+    for_each_symbol([&](auto symbol) {
+        func(move(symbol));
+        return IterationDecision::Continue;
+    });
+}
+
+template<IteratorFunction<Image::ProgramHeader> F>
 inline void Image::for_each_program_header(F func) const
 {
    auto program_header_count = this->program_header_count();
    for (unsigned i = 0; i < program_header_count; ++i) {
        if (func(program_header(i)) == IterationDecision::Break)
-            return;
+            break;
    }
 }

+template<VoidFunction<Image::ProgramHeader> F>
+inline void Image::for_each_program_header(F func) const
+{
+    for_each_program_header([&](auto header) {
+        func(move(header));
+        return IterationDecision::Continue;
+    });
+}
+
 } // end namespace ELF
--- a/Userland/Utilities/readelf.cpp
+++ b/Userland/Utilities/readelf.cpp
@ -523,7 +523,6 @@ int main(int argc, char** argv)
                printf("%08x ", section.size());
                printf("%u", section.flags());
                printf("\n");
-                return IterationDecision::Continue;
            });
        }
        printf("\n");
@ -556,8 +555,6 @@ int main(int argc, char** argv)

                if (program_header.type() == PT_INTERP)
                    printf("      [Interpreter: %s]\n", program_header.raw_data());
-
-                return IterationDecision::Continue;
            });
        }

@ -586,7 +583,6 @@ int main(int argc, char** argv)
            Vector<String> libraries;
            object->for_each_needed_library([&libraries](StringView entry) {
                libraries.append(String::formatted("{}", entry).characters());
-                return IterationDecision::Continue;
            });

            auto library_index = 0;
@ -607,7 +603,6 @@ int main(int argc, char** argv)
                } else {
                    printf("0x%08x\n", entry.val());
                }
-                return IterationDecision::Continue;
            });
        }

@ -630,7 +625,6 @@ int main(int argc, char** argv)
                    printf(" 0x%08x ", reloc.symbol().value());
                    printf(" %s", reloc.symbol().name().to_string().characters());
                    printf("\n");
-                    return IterationDecision::Continue;
                });
            }
            printf("\n");
@ -646,7 +640,6 @@ int main(int argc, char** argv)
                    printf(" 0x%08x ", reloc.symbol().value());
                    printf(" %s", reloc.symbol().name().to_string().characters());
                    printf("\n");
-                    return IterationDecision::Continue;
                });
            }
        } else {
@ -666,7 +659,7 @@ int main(int argc, char** argv)
        auto found_notes = false;
        elf_image.for_each_program_header([&found_notes](const ELF::Image::ProgramHeader& program_header) {
            if (program_header.type() != PT_NOTE)
-                return IterationDecision::Continue;
+                return;

            found_notes = true;

@ -674,8 +667,6 @@ int main(int argc, char** argv)

            // FIXME: Parse CORE notes. Notes are in JSON format on SerenityOS, but vary between systems.
            printf("%s\n", program_header.raw_data());
-
-            return IterationDecision::Continue;
        });

        if (!found_notes)
@ -714,7 +705,6 @@ int main(int argc, char** argv)
                    printf("%-8s ", object_symbol_binding_to_string(sym.bind()));
                    printf("%s", StringView(sym.name()).to_string().characters());
                    printf("\n");
-                    return IterationDecision::Continue;
                });
            }
        }
@ -738,7 +728,6 @@ int main(int argc, char** argv)
                printf("%-8s ", object_symbol_binding_to_string(sym.bind()));
                printf("%s", StringView(sym.name()).to_string().characters());
                printf("\n");
-                return IterationDecision::Continue;
            });
        } else {
            printf("Symbol table '%s' contains zero entries.\n", ELF_SYMTAB);