diff --git a/Kernel/FileSystem/OpenFileDescription.cpp b/Kernel/FileSystem/OpenFileDescription.cpp index 5b7af1e4cb..37e1a6eb0c 100644 --- a/Kernel/FileSystem/OpenFileDescription.cpp +++ b/Kernel/FileSystem/OpenFileDescription.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2018-2020, Andreas Kling + * Copyright (c) 2018-2022, Andreas Kling * Copyright (c) 2021, sin-ack * * SPDX-License-Identifier: BSD-2-Clause @@ -7,11 +7,9 @@ #include #include -#include #include #include #include -#include #include #include #include @@ -47,14 +45,15 @@ OpenFileDescription::OpenFileDescription(File& file) if (file.is_inode()) m_inode = static_cast(file).inode(); - m_is_directory = metadata().is_directory(); + auto metadata = this->metadata(); + m_state.with([&](auto& state) { state.is_directory = metadata.is_directory(); }); } OpenFileDescription::~OpenFileDescription() { m_file->detach(*this); if (is_fifo()) - static_cast(m_file.ptr())->detach(m_fifo_direction); + static_cast(m_file.ptr())->detach(fifo_direction()); // FIXME: Should this error path be observed somehow? (void)m_file->close(); if (m_inode) @@ -99,7 +98,6 @@ Thread::FileBlocker::BlockFlags OpenFileDescription::should_unblock(Thread::File ErrorOr OpenFileDescription::stat() { - MutexLocker locker(m_lock); // FIXME: This is due to the Device class not overriding File::stat(). if (m_inode) return m_inode->metadata().stat(); @@ -108,43 +106,45 @@ ErrorOr OpenFileDescription::stat() ErrorOr OpenFileDescription::seek(off_t offset, int whence) { - MutexLocker locker(m_lock); if (!m_file->is_seekable()) return ESPIPE; - off_t new_offset; + auto metadata = this->metadata(); - switch (whence) { - case SEEK_SET: - new_offset = offset; - break; - case SEEK_CUR: - if (Checked::addition_would_overflow(m_current_offset, offset)) - return EOVERFLOW; - new_offset = m_current_offset + offset; - break; - case SEEK_END: - if (!metadata().is_valid()) - return EIO; - if (Checked::addition_would_overflow(metadata().size, offset)) - return EOVERFLOW; - new_offset = metadata().size + offset; - break; - default: - return EINVAL; - } + auto new_offset = TRY(m_state.with([&](auto& state) -> ErrorOr { + off_t new_offset; + switch (whence) { + case SEEK_SET: + new_offset = offset; + break; + case SEEK_CUR: + if (Checked::addition_would_overflow(state.current_offset, offset)) + return EOVERFLOW; + new_offset = state.current_offset + offset; + break; + case SEEK_END: + if (!metadata.is_valid()) + return EIO; + if (Checked::addition_would_overflow(metadata.size, offset)) + return EOVERFLOW; + new_offset = metadata.size + offset; + break; + default: + return EINVAL; + } + if (new_offset < 0) + return EINVAL; + state.current_offset = new_offset; + return new_offset; + })); - if (new_offset < 0) - return EINVAL; // FIXME: Return EINVAL if attempting to seek past the end of a seekable device. - m_current_offset = new_offset; - m_file->did_seek(*this, new_offset); if (m_inode) m_inode->did_seek(*this, new_offset); evaluate_block_conditions(); - return m_current_offset; + return new_offset; } ErrorOr OpenFileDescription::read(UserOrKernelBuffer& buffer, u64 offset, size_t count) @@ -163,24 +163,30 @@ ErrorOr OpenFileDescription::write(u64 offset, UserOrKernelBuffer const& ErrorOr OpenFileDescription::read(UserOrKernelBuffer& buffer, size_t count) { - MutexLocker locker(m_lock); - if (Checked::addition_would_overflow(m_current_offset, count)) - return EOVERFLOW; - auto nread = TRY(m_file->read(*this, offset(), buffer, count)); + auto offset = TRY(m_state.with([&](auto& state) -> ErrorOr { + if (Checked::addition_would_overflow(state.current_offset, count)) + return EOVERFLOW; + return state.current_offset; + })); + auto nread = TRY(m_file->read(*this, offset, buffer, count)); if (m_file->is_seekable()) - m_current_offset += nread; + m_state.with([&](auto& state) { state.current_offset = offset + nread; }); evaluate_block_conditions(); return nread; } ErrorOr OpenFileDescription::write(const UserOrKernelBuffer& data, size_t size) { - MutexLocker locker(m_lock); - if (Checked::addition_would_overflow(m_current_offset, size)) - return EOVERFLOW; - auto nwritten = TRY(m_file->write(*this, offset(), data, size)); + auto offset = TRY(m_state.with([&](auto& state) -> ErrorOr { + if (Checked::addition_would_overflow(state.current_offset, size)) + return EOVERFLOW; + return state.current_offset; + })); + auto nwritten = TRY(m_file->write(*this, offset, data, size)); + if (m_file->is_seekable()) - m_current_offset += nwritten; + m_state.with([&](auto& state) { state.current_offset = offset + nwritten; }); + evaluate_block_conditions(); return nwritten; } @@ -205,7 +211,6 @@ ErrorOr> OpenFileDescription::read_entire_file() ErrorOr OpenFileDescription::get_dir_entries(UserOrKernelBuffer& output_buffer, size_t size) { - MutexLocker locker(m_lock, Mutex::Mode::Shared); if (!is_directory()) return ENOTDIR; @@ -371,19 +376,16 @@ InodeMetadata OpenFileDescription::metadata() const ErrorOr OpenFileDescription::mmap(Process& process, Memory::VirtualRange const& range, u64 offset, int prot, bool shared) { - MutexLocker locker(m_lock); return m_file->mmap(process, *this, range, offset, prot, shared); } ErrorOr OpenFileDescription::truncate(u64 length) { - MutexLocker locker(m_lock); return m_file->truncate(length); } ErrorOr OpenFileDescription::sync() { - MutexLocker locker(m_lock); return m_file->sync(); } @@ -420,22 +422,21 @@ const Socket* OpenFileDescription::socket() const void OpenFileDescription::set_file_flags(u32 flags) { - MutexLocker locker(m_lock); - m_is_blocking = !(flags & O_NONBLOCK); - m_should_append = flags & O_APPEND; - m_direct = flags & O_DIRECT; - m_file_flags = flags; + m_state.with([&](auto& state) { + state.is_blocking = !(flags & O_NONBLOCK); + state.should_append = flags & O_APPEND; + state.direct = flags & O_DIRECT; + state.file_flags = flags; + }); } ErrorOr OpenFileDescription::chmod(mode_t mode) { - MutexLocker locker(m_lock); return m_file->chmod(*this, mode); } ErrorOr OpenFileDescription::chown(UserID uid, GroupID gid) { - MutexLocker locker(m_lock); return m_file->chown(*this, uid, gid); } @@ -459,4 +460,82 @@ ErrorOr OpenFileDescription::get_flock(Userspace lock) const return m_inode->get_flock(*this, lock); } +bool OpenFileDescription::is_readable() const +{ + return m_state.with([](auto& state) { return state.readable; }); +} + +bool OpenFileDescription::is_writable() const +{ + return m_state.with([](auto& state) { return state.writable; }); +} + +void OpenFileDescription::set_readable(bool b) +{ + m_state.with([&](auto& state) { state.readable = b; }); +} + +void OpenFileDescription::set_writable(bool b) +{ + m_state.with([&](auto& state) { state.writable = b; }); +} + +void OpenFileDescription::set_rw_mode(int options) +{ + m_state.with([&](auto& state) { + state.readable = (options & O_RDONLY) == O_RDONLY; + state.writable = (options & O_WRONLY) == O_WRONLY; + }); +} + +bool OpenFileDescription::is_direct() const +{ + return m_state.with([](auto& state) { return state.direct; }); +} + +bool OpenFileDescription::is_directory() const +{ + return m_state.with([](auto& state) { return state.is_directory; }); +} + +bool OpenFileDescription::is_blocking() const +{ + return m_state.with([](auto& state) { return state.is_blocking; }); +} + +void OpenFileDescription::set_blocking(bool b) +{ + m_state.with([&](auto& state) { state.is_blocking = b; }); +} + +bool OpenFileDescription::should_append() const +{ + return m_state.with([](auto& state) { return state.should_append; }); +} + +u32 OpenFileDescription::file_flags() const +{ + return m_state.with([](auto& state) { return state.file_flags; }); +} + +FIFO::Direction OpenFileDescription::fifo_direction() const +{ + return m_state.with([](auto& state) { return state.fifo_direction; }); +} + +void OpenFileDescription::set_fifo_direction(Badge, FIFO::Direction direction) +{ + m_state.with([&](auto& state) { state.fifo_direction = direction; }); +} + +OwnPtr& OpenFileDescription::data() +{ + return m_state.with([](auto& state) -> OwnPtr& { return state.data; }); +} + +off_t OpenFileDescription::offset() const +{ + return m_state.with([](auto& state) { return state.current_offset; }); +} + } diff --git a/Kernel/FileSystem/OpenFileDescription.h b/Kernel/FileSystem/OpenFileDescription.h index fd86ea4169..acc6df00fe 100644 --- a/Kernel/FileSystem/OpenFileDescription.h +++ b/Kernel/FileSystem/OpenFileDescription.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2018-2020, Andreas Kling + * Copyright (c) 2018-2022, Andreas Kling * * SPDX-License-Identifier: BSD-2-Clause */ @@ -7,7 +7,6 @@ #pragma once #include -#include #include #include #include @@ -31,17 +30,13 @@ public: Thread::FileBlocker::BlockFlags should_unblock(Thread::FileBlocker::BlockFlags) const; - bool is_readable() const { return m_readable; } - bool is_writable() const { return m_writable; } + bool is_readable() const; + void set_readable(bool); - void set_readable(bool b) { m_readable = b; } - void set_writable(bool b) { m_writable = b; } + bool is_writable() const; + void set_writable(bool); - void set_rw_mode(int options) - { - set_readable((options & O_RDONLY) == O_RDONLY); - set_writable((options & O_WRONLY) == O_WRONLY); - } + void set_rw_mode(int options); ErrorOr close(); @@ -66,9 +61,9 @@ public: ErrorOr> original_absolute_path() const; ErrorOr> pseudo_path() const; - bool is_direct() const { return m_direct; } + bool is_direct() const; - bool is_directory() const { return m_is_directory; } + bool is_directory() const; File& file() { return *m_file; } const File& file() const { return *m_file; } @@ -98,12 +93,12 @@ public: ErrorOr mmap(Process&, Memory::VirtualRange const&, u64 offset, int prot, bool shared); - bool is_blocking() const { return m_is_blocking; } - void set_blocking(bool b) { m_is_blocking = b; } - bool should_append() const { return m_should_append; } - void set_should_append(bool s) { m_should_append = s; } + bool is_blocking() const; + void set_blocking(bool b); - u32 file_flags() const { return m_file_flags; } + bool should_append() const; + + u32 file_flags() const; void set_file_flags(u32); bool is_socket() const; @@ -112,10 +107,10 @@ public: bool is_fifo() const; FIFO* fifo(); - FIFO::Direction fifo_direction() const { return m_fifo_direction; } - void set_fifo_direction(Badge, FIFO::Direction direction) { m_fifo_direction = direction; } + FIFO::Direction fifo_direction() const; + void set_fifo_direction(Badge, FIFO::Direction direction); - OwnPtr& data() { return m_data; } + OwnPtr& data(); void set_original_inode(Badge, NonnullRefPtr&& inode) { m_inode = move(inode); } void set_original_custody(Badge, Custody& custody); @@ -123,7 +118,7 @@ public: ErrorOr truncate(u64); ErrorOr sync(); - off_t offset() const { return m_current_offset; } + off_t offset() const; ErrorOr chown(UserID, GroupID); @@ -147,21 +142,19 @@ private: RefPtr m_inode; NonnullRefPtr m_file; - off_t m_current_offset { 0 }; + struct State { + OwnPtr data; + off_t current_offset { 0 }; + u32 file_flags { 0 }; + bool readable : 1 { false }; + bool writable : 1 { false }; + bool is_blocking : 1 { true }; + bool is_directory : 1 { false }; + bool should_append : 1 { false }; + bool direct : 1 { false }; + FIFO::Direction fifo_direction : 2 { FIFO::Direction::Neither }; + }; - OwnPtr m_data; - - u32 m_file_flags { 0 }; - - bool m_readable : 1 { false }; - bool m_writable : 1 { false }; - bool m_is_blocking : 1 { true }; - bool m_is_directory : 1 { false }; - bool m_should_append : 1 { false }; - bool m_direct : 1 { false }; - FIFO::Direction m_fifo_direction { FIFO::Direction::Neither }; - - Mutex m_lock { "OpenFileDescription" }; + SpinlockProtected m_state; }; - }