From 9f6eabd73a76daa0da5521770a28e648523961a7 Mon Sep 17 00:00:00 2001 From: Brian Gianforcaro Date: Sun, 31 Oct 2021 22:47:19 -0700 Subject: Kernel: Move TTY subsystem to use KString instead of `AK::String` This is minor progress on removing the `AK::String` API from the Kernel in the interest of improving OOM safety. --- Kernel/GlobalProcessExposed.cpp | 2 +- Kernel/Syscalls/ttyname.cpp | 4 ++-- Kernel/TTY/MasterPTY.cpp | 16 ++++++++++------ Kernel/TTY/MasterPTY.h | 6 +++--- Kernel/TTY/SlavePTY.cpp | 8 ++++---- Kernel/TTY/SlavePTY.h | 6 +++--- Kernel/TTY/TTY.cpp | 2 +- Kernel/TTY/TTY.h | 2 +- Kernel/TTY/VirtualConsole.cpp | 11 ++++++++--- Kernel/TTY/VirtualConsole.h | 6 +++--- 10 files changed, 36 insertions(+), 27 deletions(-) diff --git a/Kernel/GlobalProcessExposed.cpp b/Kernel/GlobalProcessExposed.cpp index 8d158a0b69..d17d96c2f9 100644 --- a/Kernel/GlobalProcessExposed.cpp +++ b/Kernel/GlobalProcessExposed.cpp @@ -497,7 +497,7 @@ private: process_object.add("nfds", process.fds().open_count()); process_object.add("name", process.name()); process_object.add("executable", process.executable() ? process.executable()->absolute_path() : ""); - process_object.add("tty", process.tty() ? process.tty()->tty_name() : "notty"); + process_object.add("tty", process.tty() ? process.tty()->tty_name().view() : "notty"sv); process_object.add("amount_virtual", process.address_space().amount_virtual()); process_object.add("amount_resident", process.address_space().amount_resident()); process_object.add("amount_dirty_private", process.address_space().amount_dirty_private()); diff --git a/Kernel/Syscalls/ttyname.cpp b/Kernel/Syscalls/ttyname.cpp index ca4df4de74..2e73d06a9c 100644 --- a/Kernel/Syscalls/ttyname.cpp +++ b/Kernel/Syscalls/ttyname.cpp @@ -18,7 +18,7 @@ KResultOr Process::sys$ttyname(int fd, Userspace buffer, size_t auto description = TRY(fds().open_file_description(fd)); if (!description->is_tty()) return ENOTTY; - auto tty_name = description->tty()->tty_name(); + auto& tty_name = description->tty()->tty_name(); if (size < tty_name.length() + 1) return ERANGE; return copy_to_user(buffer, tty_name.characters(), tty_name.length() + 1); @@ -32,7 +32,7 @@ KResultOr Process::sys$ptsname(int fd, Userspace buffer, size_t auto* master_pty = description->master_pty(); if (!master_pty) return ENOTTY; - auto pts_name = master_pty->pts_name(); + auto& pts_name = master_pty->pts_name(); if (size < pts_name.length() + 1) return ERANGE; return copy_to_user(buffer, pts_name.characters(), pts_name.length() + 1); diff --git a/Kernel/TTY/MasterPTY.cpp b/Kernel/TTY/MasterPTY.cpp index ba5e2db5c2..7f1680bb82 100644 --- a/Kernel/TTY/MasterPTY.cpp +++ b/Kernel/TTY/MasterPTY.cpp @@ -18,21 +18,25 @@ namespace Kernel { KResultOr> MasterPTY::try_create(unsigned int index) { + // FIXME: Don't make a temporary String here + auto pts_name = TRY(KString::try_create(String::formatted("/dev/pts/{}", index))); + auto tty_name = TRY(pts_name->try_clone()); + auto buffer = TRY(DoubleBuffer::try_create()); - auto master_pty = TRY(adopt_nonnull_ref_or_enomem(new (nothrow) MasterPTY(index, move(buffer)))); - auto slave_pty = TRY(adopt_nonnull_ref_or_enomem(new (nothrow) SlavePTY(*master_pty, index))); + auto master_pty = TRY(adopt_nonnull_ref_or_enomem(new (nothrow) MasterPTY(index, move(buffer), move(pts_name)))); + auto slave_pty = TRY(adopt_nonnull_ref_or_enomem(new (nothrow) SlavePTY(*master_pty, index, move(tty_name)))); master_pty->m_slave = slave_pty; master_pty->after_inserting(); slave_pty->after_inserting(); return master_pty; } -MasterPTY::MasterPTY(unsigned index, NonnullOwnPtr buffer) +MasterPTY::MasterPTY(unsigned index, NonnullOwnPtr buffer, NonnullOwnPtr pts_name) : CharacterDevice(200, index) , m_index(index) , m_buffer(move(buffer)) + , m_pts_name(move(pts_name)) { - m_pts_name = String::formatted("/dev/pts/{}", m_index); auto& process = Process::current(); set_uid(process.uid()); set_gid(process.gid()); @@ -49,9 +53,9 @@ MasterPTY::~MasterPTY() PTYMultiplexer::the().notify_master_destroyed({}, m_index); } -String MasterPTY::pts_name() const +KString const& MasterPTY::pts_name() const { - return m_pts_name; + return *m_pts_name; } KResultOr MasterPTY::read(OpenFileDescription&, u64, UserOrKernelBuffer& buffer, size_t size) diff --git a/Kernel/TTY/MasterPTY.h b/Kernel/TTY/MasterPTY.h index 38a9cbc747..cf5f1da616 100644 --- a/Kernel/TTY/MasterPTY.h +++ b/Kernel/TTY/MasterPTY.h @@ -20,7 +20,7 @@ public: virtual ~MasterPTY() override; unsigned index() const { return m_index; } - String pts_name() const; + KString const& pts_name() const; KResultOr on_slave_write(const UserOrKernelBuffer&, size_t); bool can_write_from_slave() const; void notify_slave_closed(Badge); @@ -29,7 +29,7 @@ public: virtual KResultOr> pseudo_path(const OpenFileDescription&) const override; private: - explicit MasterPTY(unsigned index, NonnullOwnPtr buffer); + explicit MasterPTY(unsigned index, NonnullOwnPtr buffer, NonnullOwnPtr pts_name); // ^CharacterDevice virtual KResultOr read(OpenFileDescription&, u64, UserOrKernelBuffer&, size_t) override; virtual KResultOr write(OpenFileDescription&, u64, const UserOrKernelBuffer&, size_t) override; @@ -44,7 +44,7 @@ private: unsigned m_index; bool m_closed { false }; NonnullOwnPtr m_buffer; - String m_pts_name; + NonnullOwnPtr m_pts_name; }; } diff --git a/Kernel/TTY/SlavePTY.cpp b/Kernel/TTY/SlavePTY.cpp index e06ff6949c..738c777e2e 100644 --- a/Kernel/TTY/SlavePTY.cpp +++ b/Kernel/TTY/SlavePTY.cpp @@ -35,12 +35,12 @@ bool SlavePTY::unref() const return did_hit_zero; } -SlavePTY::SlavePTY(MasterPTY& master, unsigned index) +SlavePTY::SlavePTY(MasterPTY& master, unsigned index, NonnullOwnPtr tty_name) : TTY(201, index) , m_master(master) , m_index(index) + , m_tty_name(move(tty_name)) { - m_tty_name = String::formatted("/dev/pts/{}", m_index); auto& process = Process::current(); set_uid(process.uid()); set_gid(process.gid()); @@ -54,9 +54,9 @@ SlavePTY::~SlavePTY() dbgln_if(SLAVEPTY_DEBUG, "~SlavePTY({})", m_index); } -String const& SlavePTY::tty_name() const +KString const& SlavePTY::tty_name() const { - return m_tty_name; + return *m_tty_name; } void SlavePTY::echo(u8 ch) diff --git a/Kernel/TTY/SlavePTY.h b/Kernel/TTY/SlavePTY.h index e10795318c..c2928a3e25 100644 --- a/Kernel/TTY/SlavePTY.h +++ b/Kernel/TTY/SlavePTY.h @@ -27,7 +27,7 @@ public: private: // ^TTY - virtual String const& tty_name() const override; + virtual KString const& tty_name() const override; virtual KResultOr on_tty_write(const UserOrKernelBuffer&, size_t) override; virtual void echo(u8) override; @@ -39,12 +39,12 @@ private: virtual KResult close() override; friend class MasterPTY; - SlavePTY(MasterPTY&, unsigned index); + SlavePTY(MasterPTY&, unsigned index, NonnullOwnPtr pts_name); RefPtr m_master; time_t m_time_of_last_write { 0 }; unsigned m_index { 0 }; - String m_tty_name; + NonnullOwnPtr m_tty_name; mutable IntrusiveListNode m_list_node; diff --git a/Kernel/TTY/TTY.cpp b/Kernel/TTY/TTY.cpp index 7e4c2f26c8..dc428433ab 100644 --- a/Kernel/TTY/TTY.cpp +++ b/Kernel/TTY/TTY.cpp @@ -578,7 +578,7 @@ KResult TTY::ioctl(OpenFileDescription&, unsigned request, Userspace arg) KResultOr> TTY::pseudo_path(const OpenFileDescription&) const { - return KString::try_create(tty_name()); + return tty_name().try_clone(); } void TTY::set_size(unsigned short columns, unsigned short rows) diff --git a/Kernel/TTY/TTY.h b/Kernel/TTY/TTY.h index 3d2cc760a2..12f7f3ac26 100644 --- a/Kernel/TTY/TTY.h +++ b/Kernel/TTY/TTY.h @@ -28,7 +28,7 @@ public: virtual KResult ioctl(OpenFileDescription&, unsigned request, Userspace arg) override final; virtual KResultOr> pseudo_path(const OpenFileDescription&) const override; - virtual String const& tty_name() const = 0; + virtual KString const& tty_name() const = 0; unsigned short rows() const { return m_rows; } unsigned short columns() const { return m_columns; } diff --git a/Kernel/TTY/VirtualConsole.cpp b/Kernel/TTY/VirtualConsole.cpp index e4992d88c1..9a6d0ddf11 100644 --- a/Kernel/TTY/VirtualConsole.cpp +++ b/Kernel/TTY/VirtualConsole.cpp @@ -103,7 +103,12 @@ void VirtualConsole::set_graphical(bool graphical) UNMAP_AFTER_INIT NonnullRefPtr VirtualConsole::create(size_t index) { - auto virtual_console_or_error = DeviceManagement::try_create_device(index); + // FIXME: Don't make a temporary String here + auto pts_name_or_error = KString::try_create(String::formatted("/dev/tty/{}", index)); + VERIFY(!pts_name_or_error.is_error()); + auto pts_name = pts_name_or_error.release_value(); + + auto virtual_console_or_error = DeviceManagement::try_create_device(index, move(pts_name)); // FIXME: Find a way to propagate errors VERIFY(!virtual_console_or_error.is_error()); return virtual_console_or_error.release_value(); @@ -123,7 +128,6 @@ UNMAP_AFTER_INIT NonnullRefPtr VirtualConsole::create_with_prese UNMAP_AFTER_INIT void VirtualConsole::initialize() { - m_tty_name = String::formatted("/dev/tty{}", m_index); VERIFY(GraphicsManagement::the().console()); set_size(GraphicsManagement::the().console()->max_column(), GraphicsManagement::the().console()->max_row()); m_console_impl.set_size(GraphicsManagement::the().console()->max_column(), GraphicsManagement::the().console()->max_row()); @@ -174,9 +178,10 @@ void VirtualConsole::refresh_after_resolution_change() flush_dirty_lines(); } -UNMAP_AFTER_INIT VirtualConsole::VirtualConsole(const unsigned index) +UNMAP_AFTER_INIT VirtualConsole::VirtualConsole(const unsigned index, NonnullOwnPtr tty_name) : TTY(4, index) , m_index(index) + , m_tty_name(move(tty_name)) , m_console_impl(*this) { initialize(); diff --git a/Kernel/TTY/VirtualConsole.h b/Kernel/TTY/VirtualConsole.h index 500f6a2a1a..d29d123a21 100644 --- a/Kernel/TTY/VirtualConsole.h +++ b/Kernel/TTY/VirtualConsole.h @@ -86,13 +86,13 @@ public: void emit_char(char); private: - explicit VirtualConsole(const unsigned index); + explicit VirtualConsole(const unsigned index, NonnullOwnPtr tty_name); // ^KeyboardClient virtual void on_key_pressed(KeyEvent) override; // ^TTY virtual KResultOr on_tty_write(const UserOrKernelBuffer&, size_t) override; - virtual String const& tty_name() const override { return m_tty_name; } + virtual KString const& tty_name() const override { return *m_tty_name; } virtual void echo(u8) override; // ^TerminalClient @@ -114,7 +114,7 @@ private: bool m_active { false }; bool m_graphical { false }; - String m_tty_name; + NonnullOwnPtr m_tty_name; private: void initialize(); -- cgit v1.2.3