From 0de8c95d49e9c5042c4e4581a2483266586064e9 Mon Sep 17 00:00:00 2001
From: Andreas Kling <kling@serenityos.org>
Date: Mon, 16 Aug 2021 22:13:58 +0200
Subject: [PATCH] Kernel: Convert SlavePTY all-instances HashTable to an
 IntrusiveList

This simplifies the DevPtsFS implementation somewhat, as it no longer
requires SlavePTY to register itself with it, since it can now simply
use the list of SlavePTY instances.
---
 Kernel/FileSystem/DevPtsFS.cpp | 47 +++++++++++++++-------------------
 Kernel/FileSystem/DevPtsFS.h   |  3 ---
 Kernel/TTY/SlavePTY.cpp        | 27 ++++++++++++++++---
 Kernel/TTY/SlavePTY.h          |  9 ++++++-
 4 files changed, 53 insertions(+), 33 deletions(-)

diff --git a/Kernel/FileSystem/DevPtsFS.cpp b/Kernel/FileSystem/DevPtsFS.cpp
index 7c84db9f0b..136a39afa2 100644
--- a/Kernel/FileSystem/DevPtsFS.cpp
+++ b/Kernel/FileSystem/DevPtsFS.cpp
@@ -1,11 +1,10 @@
 /*
  * Copyright (c) 2019-2020, Sergey Bugaev <bugaevc@serenityos.org>
+ * Copyright (c) 2021, Andreas Kling <kling@serenityos.org>
  *
  * SPDX-License-Identifier: BSD-2-Clause
  */
 
-#include <AK/Singleton.h>
-#include <AK/StringView.h>
 #include <Kernel/FileSystem/DevPtsFS.h>
 #include <Kernel/FileSystem/VirtualFileSystem.h>
 #include <Kernel/TTY/SlavePTY.h>
@@ -25,8 +24,6 @@ DevPtsFS::~DevPtsFS()
 {
 }
 
-static Singleton<HashTable<unsigned>> s_ptys;
-
 KResult DevPtsFS::initialize()
 {
     m_root_inode = adopt_ref_if_nonnull(new (nothrow) DevPtsFSInode(*this, 1, nullptr));
@@ -80,16 +77,6 @@ RefPtr<Inode> DevPtsFS::get_inode(InodeIdentifier inode_id) const
     return inode;
 }
 
-void DevPtsFS::register_slave_pty(SlavePTY& slave_pty)
-{
-    s_ptys->set(slave_pty.index());
-}
-
-void DevPtsFS::unregister_slave_pty(SlavePTY& slave_pty)
-{
-    s_ptys->remove(slave_pty.index());
-}
-
 DevPtsFSInode::DevPtsFSInode(DevPtsFS& fs, InodeIndex index, SlavePTY* pty)
     : Inode(fs, index)
 {
@@ -129,11 +116,13 @@ KResult DevPtsFSInode::traverse_as_directory(Function<bool(FileSystem::Directory
     callback({ ".", identifier(), 0 });
     callback({ "..", identifier(), 0 });
 
-    for (unsigned pty_index : *s_ptys) {
-        String name = String::number(pty_index);
-        InodeIdentifier identifier = { fsid(), pty_index_to_inode_index(pty_index) };
-        callback({ name, identifier, 0 });
-    }
+    SlavePTY::all_instances().with([&](auto& list) {
+        for (SlavePTY& slave_pty : list) {
+            String name = String::number(slave_pty.index());
+            InodeIdentifier identifier = { fsid(), pty_index_to_inode_index(slave_pty.index()) };
+            callback({ name, identifier, 0 });
+        }
+    });
 
     return KSuccess;
 }
@@ -146,14 +135,20 @@ KResultOr<NonnullRefPtr<Inode>> DevPtsFSInode::lookup(StringView name)
         return *this;
 
     auto pty_index = name.to_uint();
-    if (pty_index.has_value() && s_ptys->contains(pty_index.value())) {
-        auto inode = fs().get_inode({ fsid(), pty_index_to_inode_index(pty_index.value()) });
-        if (!inode)
-            return ENOENT;
-        return inode.release_nonnull();
-    }
+    if (!pty_index.has_value())
+        return ENOENT;
 
-    return ENOENT;
+    return SlavePTY::all_instances().with([&](auto& list) -> KResultOr<NonnullRefPtr<Inode>> {
+        for (SlavePTY& slave_pty : list) {
+            if (slave_pty.index() != pty_index.value())
+                continue;
+            auto inode = fs().get_inode({ fsid(), pty_index_to_inode_index(pty_index.value()) });
+            if (!inode)
+                return ENOENT;
+            return inode.release_nonnull();
+        }
+        return ENOENT;
+    });
 }
 
 void DevPtsFSInode::flush_metadata()
diff --git a/Kernel/FileSystem/DevPtsFS.h b/Kernel/FileSystem/DevPtsFS.h
index 568b2c2ca4..d89ab1dfe4 100644
--- a/Kernel/FileSystem/DevPtsFS.h
+++ b/Kernel/FileSystem/DevPtsFS.h
@@ -27,9 +27,6 @@ public:
 
     virtual Inode& root_inode() override;
 
-    static void register_slave_pty(SlavePTY&);
-    static void unregister_slave_pty(SlavePTY&);
-
 private:
     DevPtsFS();
     RefPtr<Inode> get_inode(InodeIdentifier) const;
diff --git a/Kernel/TTY/SlavePTY.cpp b/Kernel/TTY/SlavePTY.cpp
index 461e90e915..664a3050f1 100644
--- a/Kernel/TTY/SlavePTY.cpp
+++ b/Kernel/TTY/SlavePTY.cpp
@@ -1,9 +1,10 @@
 /*
- * Copyright (c) 2018-2020, Andreas Kling <kling@serenityos.org>
+ * Copyright (c) 2018-2021, Andreas Kling <kling@serenityos.org>
  *
  * SPDX-License-Identifier: BSD-2-Clause
  */
 
+#include <AK/Singleton.h>
 #include <Kernel/Debug.h>
 #include <Kernel/FileSystem/DevPtsFS.h>
 #include <Kernel/Process.h>
@@ -12,6 +13,26 @@
 
 namespace Kernel {
 
+static Singleton<SpinLockProtectedValue<SlavePTY::List>> s_all_instances;
+
+SpinLockProtectedValue<SlavePTY::List>& SlavePTY::all_instances()
+{
+    return s_all_instances;
+}
+
+bool SlavePTY::unref() const
+{
+    bool did_hit_zero = SlavePTY::all_instances().with([&](auto&) {
+        if (deref_base())
+            return false;
+        m_list_node.remove();
+        return true;
+    });
+    if (did_hit_zero)
+        delete this;
+    return did_hit_zero;
+}
+
 SlavePTY::SlavePTY(MasterPTY& master, unsigned index)
     : TTY(201, index)
     , m_master(master)
@@ -21,14 +42,14 @@ SlavePTY::SlavePTY(MasterPTY& master, unsigned index)
     auto process = Process::current();
     set_uid(process->uid());
     set_gid(process->gid());
-    DevPtsFS::register_slave_pty(*this);
     set_size(80, 25);
+
+    SlavePTY::all_instances().with([&](auto& list) { list.append(*this); });
 }
 
 SlavePTY::~SlavePTY()
 {
     dbgln_if(SLAVEPTY_DEBUG, "~SlavePTY({})", m_index);
-    DevPtsFS::unregister_slave_pty(*this);
 }
 
 String const& SlavePTY::tty_name() const
diff --git a/Kernel/TTY/SlavePTY.h b/Kernel/TTY/SlavePTY.h
index 0539042afe..d8c44659c4 100644
--- a/Kernel/TTY/SlavePTY.h
+++ b/Kernel/TTY/SlavePTY.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018-2020, Andreas Kling <kling@serenityos.org>
+ * Copyright (c) 2018-2021, Andreas Kling <kling@serenityos.org>
  *
  * SPDX-License-Identifier: BSD-2-Clause
  */
@@ -15,6 +15,7 @@ class MasterPTY;
 
 class SlavePTY final : public TTY {
 public:
+    virtual bool unref() const override;
     virtual ~SlavePTY() override;
 
     void on_master_write(const UserOrKernelBuffer&, size_t);
@@ -47,6 +48,12 @@ private:
     time_t m_time_of_last_write { 0 };
     unsigned m_index { 0 };
     String m_tty_name;
+
+    mutable IntrusiveListNode<SlavePTY> m_list_node;
+
+public:
+    using List = IntrusiveList<SlavePTY, RawPtr<SlavePTY>, &SlavePTY::m_list_node>;
+    static SpinLockProtectedValue<SlavePTY::List>& all_instances();
 };
 
 }