From 752de9cd27ee336ee6df4f6bcf6cb2cb8fc39eea Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Sun, 11 Aug 2019 09:32:21 +0200 Subject: [PATCH] FileDescription: Disallow construction with a null File It's not valid for a FileDescription to not have a file, so let's disallow it by taking a File& (or FIFO&) in the constructor. --- Kernel/FileSystem/FIFO.cpp | 2 +- Kernel/FileSystem/File.cpp | 2 +- Kernel/FileSystem/FileDescription.cpp | 19 +++++++++---------- Kernel/FileSystem/FileDescription.h | 8 ++++---- Kernel/Process.cpp | 4 ++-- Kernel/TTY/PTYMultiplexer.cpp | 2 +- 6 files changed, 18 insertions(+), 19 deletions(-) diff --git a/Kernel/FileSystem/FIFO.cpp b/Kernel/FileSystem/FIFO.cpp index 7f00e4064d..6fc77a7b2b 100644 --- a/Kernel/FileSystem/FIFO.cpp +++ b/Kernel/FileSystem/FIFO.cpp @@ -32,7 +32,7 @@ NonnullRefPtr FIFO::create(uid_t uid) NonnullRefPtr FIFO::open_direction(FIFO::Direction direction) { - auto description = FileDescription::create(this); + auto description = FileDescription::create(*this); attach(direction); description->set_fifo_direction({}, direction); return description; diff --git a/Kernel/FileSystem/File.cpp b/Kernel/FileSystem/File.cpp index 6f12224027..56b7305cf3 100644 --- a/Kernel/FileSystem/File.cpp +++ b/Kernel/FileSystem/File.cpp @@ -12,7 +12,7 @@ File::~File() KResultOr> File::open(int options) { UNUSED_PARAM(options); - return FileDescription::create(this); + return FileDescription::create(*this); } void File::close() diff --git a/Kernel/FileSystem/FileDescription.cpp b/Kernel/FileSystem/FileDescription.cpp index 9a02da8f8f..d9de69a56a 100644 --- a/Kernel/FileSystem/FileDescription.cpp +++ b/Kernel/FileSystem/FileDescription.cpp @@ -15,23 +15,23 @@ #include #include -NonnullRefPtr FileDescription::create(RefPtr&& custody) +NonnullRefPtr FileDescription::create(Custody& custody) { - auto description = adopt(*new FileDescription(InodeFile::create(custody->inode()))); - description->m_custody = move(custody); + auto description = adopt(*new FileDescription(InodeFile::create(custody.inode()))); + description->m_custody = custody; return description; } -NonnullRefPtr FileDescription::create(RefPtr file, SocketRole role) +NonnullRefPtr FileDescription::create(File& file, SocketRole role) { - return adopt(*new FileDescription(move(file), role)); + return adopt(*new FileDescription(file, role)); } -FileDescription::FileDescription(RefPtr&& file, SocketRole role) - : m_file(move(file)) +FileDescription::FileDescription(File& file, SocketRole role) + : m_file(file) { - if (m_file->is_inode()) - m_inode = static_cast(*m_file).inode(); + if (file.is_inode()) + m_inode = static_cast(file).inode(); set_socket_role(role); } @@ -42,7 +42,6 @@ FileDescription::~FileDescription() if (is_fifo()) static_cast(m_file.ptr())->detach(m_fifo_direction); m_file->close(); - m_file = nullptr; m_inode = nullptr; } diff --git a/Kernel/FileSystem/FileDescription.h b/Kernel/FileSystem/FileDescription.h index 80cf993f2b..38b36a3d76 100644 --- a/Kernel/FileSystem/FileDescription.h +++ b/Kernel/FileSystem/FileDescription.h @@ -22,8 +22,8 @@ class SharedMemory; class FileDescription : public RefCounted { public: - static NonnullRefPtr create(RefPtr&&); - static NonnullRefPtr create(RefPtr, SocketRole = SocketRole::None); + static NonnullRefPtr create(Custody&); + static NonnullRefPtr create(File&, SocketRole = SocketRole::None); ~FileDescription(); NonnullRefPtr clone(); @@ -106,12 +106,12 @@ public: private: friend class VFS; - FileDescription(RefPtr&&, SocketRole = SocketRole::None); + FileDescription(File&, SocketRole = SocketRole::None); FileDescription(FIFO&, FIFO::Direction); RefPtr m_custody; RefPtr m_inode; - RefPtr m_file; + NonnullRefPtr m_file; off_t m_current_offset { 0 }; diff --git a/Kernel/Process.cpp b/Kernel/Process.cpp index beb3f7024a..26554c676c 100644 --- a/Kernel/Process.cpp +++ b/Kernel/Process.cpp @@ -2169,7 +2169,7 @@ int Process::sys$accept(int accepting_socket_fd, sockaddr* address, socklen_t* a ASSERT(accepted_socket); bool success = accepted_socket->get_peer_address(address, address_size); ASSERT(success); - auto accepted_socket_description = FileDescription::create(move(accepted_socket), SocketRole::Accepted); + auto accepted_socket_description = FileDescription::create(*accepted_socket, SocketRole::Accepted); // NOTE: The accepted socket inherits fd flags from the accepting socket. // I'm not sure if this matches other systems but it makes sense to me. accepted_socket_description->set_blocking(accepting_socket_description->is_blocking()); @@ -2652,7 +2652,7 @@ int Process::sys$shm_open(const char* name, int flags, mode_t mode) auto shm_or_error = SharedMemory::open(String(name), flags, mode); if (shm_or_error.is_error()) return shm_or_error.error(); - auto description = FileDescription::create(shm_or_error.value().ptr()); + auto description = FileDescription::create(shm_or_error.value()); m_fds[fd].set(move(description), FD_CLOEXEC); return fd; } diff --git a/Kernel/TTY/PTYMultiplexer.cpp b/Kernel/TTY/PTYMultiplexer.cpp index 6a9b645e0c..ee11109a9c 100644 --- a/Kernel/TTY/PTYMultiplexer.cpp +++ b/Kernel/TTY/PTYMultiplexer.cpp @@ -39,7 +39,7 @@ KResultOr> PTYMultiplexer::open(int options) #ifdef PTMX_DEBUG dbgprintf("PTYMultiplexer::open: Vending master %u\n", master->index()); #endif - return FileDescription::create(master.ptr()); + return FileDescription::create(move(master)); } void PTYMultiplexer::notify_master_destroyed(Badge, unsigned index)