From 5f6ab773520c71bd32cad4ca77d86a6bb280c8c7 Mon Sep 17 00:00:00 2001 From: Brian Gianforcaro Date: Sun, 7 Mar 2021 03:01:11 -0800 Subject: [PATCH] Kernel: Add bitwise operators for Thread::FileBlocker::BlockFlags enum Switch to using type-safe bitwise operators for the BlockFlags class, this cleans up a lot of boilerplate casts which are necessary when the enum is declared as `enum class`. --- Kernel/FileSystem/FileDescription.cpp | 23 +++++++------ Kernel/Net/IPv4Socket.cpp | 10 +++--- Kernel/Net/LocalSocket.cpp | 2 +- Kernel/Syscalls/read.cpp | 10 +++--- Kernel/Syscalls/select.cpp | 48 ++++++++++++++------------- Kernel/Thread.h | 3 ++ Kernel/ThreadBlockers.cpp | 8 ++--- 7 files changed, 57 insertions(+), 47 deletions(-) diff --git a/Kernel/FileSystem/FileDescription.cpp b/Kernel/FileSystem/FileDescription.cpp index b7df9bc6598..f66811d450d 100644 --- a/Kernel/FileSystem/FileDescription.cpp +++ b/Kernel/FileSystem/FileDescription.cpp @@ -98,22 +98,23 @@ KResult FileDescription::attach() Thread::FileBlocker::BlockFlags FileDescription::should_unblock(Thread::FileBlocker::BlockFlags block_flags) const { - u32 unblock_flags = (u32)Thread::FileBlocker::BlockFlags::None; - if (((u32)block_flags & (u32)Thread::FileBlocker::BlockFlags::Read) && can_read()) - unblock_flags |= (u32)Thread::FileBlocker::BlockFlags::Read; - if (((u32)block_flags & (u32)Thread::FileBlocker::BlockFlags::Write) && can_write()) - unblock_flags |= (u32)Thread::FileBlocker::BlockFlags::Write; + using BlockFlags = Thread::FileBlocker::BlockFlags; + BlockFlags unblock_flags = BlockFlags::None; + if (has_flag(block_flags, BlockFlags::Read) && can_read()) + unblock_flags |= BlockFlags::Read; + if (has_flag(block_flags, BlockFlags::Write) && can_write()) + unblock_flags |= BlockFlags::Write; // TODO: Implement Thread::FileBlocker::BlockFlags::Exception - if ((u32)block_flags & (u32)Thread::FileBlocker::BlockFlags::SocketFlags) { + if (has_flag(block_flags, BlockFlags::SocketFlags)) { auto* sock = socket(); VERIFY(sock); - if (((u32)block_flags & (u32)Thread::FileBlocker::BlockFlags::Accept) && sock->can_accept()) - unblock_flags |= (u32)Thread::FileBlocker::BlockFlags::Accept; - if (((u32)block_flags & (u32)Thread::FileBlocker::BlockFlags::Connect) && sock->setup_state() == Socket::SetupState::Completed) - unblock_flags |= (u32)Thread::FileBlocker::BlockFlags::Connect; + if (has_flag(block_flags, BlockFlags::Accept) && sock->can_accept()) + unblock_flags |= BlockFlags::Accept; + if (has_flag(block_flags, BlockFlags::Connect) && sock->setup_state() == Socket::SetupState::Completed) + unblock_flags |= BlockFlags::Connect; } - return (Thread::FileBlocker::BlockFlags)unblock_flags; + return unblock_flags; } KResult FileDescription::stat(::stat& buffer) diff --git a/Kernel/Net/IPv4Socket.cpp b/Kernel/Net/IPv4Socket.cpp index 104fcc6b8aa..3313ea4e752 100644 --- a/Kernel/Net/IPv4Socket.cpp +++ b/Kernel/Net/IPv4Socket.cpp @@ -47,6 +47,8 @@ namespace Kernel { static AK::Singleton>> s_table; +using BlockFlags = Thread::FileDescriptionBlocker::BlockFlags; + Lockable>& IPv4Socket::all_sockets() { return *s_table; @@ -247,11 +249,11 @@ KResultOr IPv4Socket::receive_byte_buffered(FileDescription& description return EAGAIN; locker.unlock(); - auto unblocked_flags = Thread::FileDescriptionBlocker::BlockFlags::None; + auto unblocked_flags = BlockFlags::None; auto res = Thread::current()->block({}, description, unblocked_flags); locker.lock(); - if (!((u32)unblocked_flags & (u32)Thread::FileDescriptionBlocker::BlockFlags::Read)) { + if (!has_flag(unblocked_flags, BlockFlags::Read)) { if (res.was_interrupted()) return EINTR; @@ -300,11 +302,11 @@ KResultOr IPv4Socket::receive_packet_buffered(FileDescription& descripti } locker.unlock(); - auto unblocked_flags = Thread::FileDescriptionBlocker::BlockFlags::None; + auto unblocked_flags = BlockFlags::None; auto res = Thread::current()->block({}, description, unblocked_flags); locker.lock(); - if (!((u32)unblocked_flags & (u32)Thread::FileDescriptionBlocker::BlockFlags::Read)) { + if (!has_flag(unblocked_flags, BlockFlags::Read)) { if (res.was_interrupted()) return EINTR; diff --git a/Kernel/Net/LocalSocket.cpp b/Kernel/Net/LocalSocket.cpp index 09809cf5ace..6dd94e81900 100644 --- a/Kernel/Net/LocalSocket.cpp +++ b/Kernel/Net/LocalSocket.cpp @@ -192,7 +192,7 @@ KResult LocalSocket::connect(FileDescription& description, Userspace Process::sys$readv(int fd, Userspace iov, int iov_count) { REQUIRE_PROMISE(stdio); @@ -65,10 +67,10 @@ KResultOr Process::sys$readv(int fd, Userspace iov for (auto& vec : vecs) { if (description->is_blocking()) { if (!description->can_read()) { - auto unblock_flags = Thread::FileBlocker::BlockFlags::None; + auto unblock_flags = BlockFlags::None; if (Thread::current()->block({}, *description, unblock_flags).was_interrupted()) return EINTR; - if (!((u32)unblock_flags & (u32)Thread::FileBlocker::BlockFlags::Read)) + if (!has_flag(unblock_flags, BlockFlags::Read)) return EAGAIN; // TODO: handle exceptions in unblock_flags } @@ -102,10 +104,10 @@ KResultOr Process::sys$read(int fd, Userspace buffer, ssize_t size return EISDIR; if (description->is_blocking()) { if (!description->can_read()) { - auto unblock_flags = Thread::FileBlocker::BlockFlags::None; + auto unblock_flags = BlockFlags::None; if (Thread::current()->block({}, *description, unblock_flags).was_interrupted()) return EINTR; - if (!((u32)unblock_flags & (u32)Thread::FileBlocker::BlockFlags::Read)) + if (!has_flag(unblock_flags, BlockFlags::Read)) return EAGAIN; // TODO: handle exceptions in unblock_flags } diff --git a/Kernel/Syscalls/select.cpp b/Kernel/Syscalls/select.cpp index def1cc2c957..a03710dc656 100644 --- a/Kernel/Syscalls/select.cpp +++ b/Kernel/Syscalls/select.cpp @@ -32,6 +32,8 @@ namespace Kernel { +using BlockFlags = Thread::FileBlocker::BlockFlags; + KResultOr Process::sys$select(Userspace user_params) { REQUIRE_PROMISE(stdio); @@ -76,14 +78,14 @@ KResultOr Process::sys$select(Userspace u Thread::SelectBlocker::FDVector fds_info; Vector fds; for (int fd = 0; fd < params.nfds; fd++) { - u32 block_flags = (u32)Thread::FileBlocker::BlockFlags::None; + auto block_flags = BlockFlags::None; if (params.readfds && FD_ISSET(fd, &fds_read)) - block_flags |= (u32)Thread::FileBlocker::BlockFlags::Read; + block_flags |= BlockFlags::Read; if (params.writefds && FD_ISSET(fd, &fds_write)) - block_flags |= (u32)Thread::FileBlocker::BlockFlags::Write; + block_flags |= BlockFlags::Write; if (params.exceptfds && FD_ISSET(fd, &fds_except)) - block_flags |= (u32)Thread::FileBlocker::BlockFlags::Exception; - if (block_flags == (u32)Thread::FileBlocker::BlockFlags::None) + block_flags |= BlockFlags::Exception; + if (block_flags == BlockFlags::None) continue; auto description = file_description(fd); @@ -91,7 +93,7 @@ KResultOr Process::sys$select(Userspace u dbgln("sys$select: Bad fd number {}", fd); return EBADF; } - fds_info.append({ description.release_nonnull(), (Thread::FileBlocker::BlockFlags)block_flags }); + fds_info.append({ description.release_nonnull(), block_flags }); fds.append(fd); } @@ -113,17 +115,17 @@ KResultOr Process::sys$select(Userspace u int marked_fd_count = 0; for (size_t i = 0; i < fds_info.size(); i++) { auto& fd_entry = fds_info[i]; - if (fd_entry.unblocked_flags == Thread::FileBlocker::BlockFlags::None) + if (fd_entry.unblocked_flags == BlockFlags::None) continue; - if (params.readfds && ((u32)fd_entry.unblocked_flags & (u32)Thread::FileBlocker::BlockFlags::Read)) { + if (params.readfds && has_flag(fd_entry.unblocked_flags, BlockFlags::Read)) { FD_SET(fds[i], &fds_read); marked_fd_count++; } - if (params.writefds && ((u32)fd_entry.unblocked_flags & (u32)Thread::FileBlocker::BlockFlags::Write)) { + if (params.writefds && has_flag(fd_entry.unblocked_flags, BlockFlags::Write)) { FD_SET(fds[i], &fds_write); marked_fd_count++; } - if (params.exceptfds && ((u32)fd_entry.unblocked_flags & (u32)Thread::FileBlocker::BlockFlags::Exception)) { + if (params.exceptfds && has_flag(fd_entry.unblocked_flags, BlockFlags::Exception)) { FD_SET(fds[i], &fds_except); marked_fd_count++; } @@ -180,14 +182,14 @@ KResultOr Process::sys$poll(Userspace user_ dbgln("sys$poll: Bad fd number {}", pfd.fd); return EBADF; } - u32 block_flags = (u32)Thread::FileBlocker::BlockFlags::Exception; // always want POLLERR, POLLHUP, POLLNVAL + BlockFlags block_flags = BlockFlags::Exception; // always want POLLERR, POLLHUP, POLLNVAL if (pfd.events & POLLIN) - block_flags |= (u32)Thread::FileBlocker::BlockFlags::Read; + block_flags |= BlockFlags::Read; if (pfd.events & POLLOUT) - block_flags |= (u32)Thread::FileBlocker::BlockFlags::Write; + block_flags |= BlockFlags::Write; if (pfd.events & POLLPRI) - block_flags |= (u32)Thread::FileBlocker::BlockFlags::ReadPriority; - fds_info.append({ description.release_nonnull(), (Thread::FileBlocker::BlockFlags)block_flags }); + block_flags |= BlockFlags::ReadPriority; + fds_info.append({ description.release_nonnull(), block_flags }); } auto current_thread = Thread::current(); @@ -213,26 +215,26 @@ KResultOr Process::sys$poll(Userspace user_ auto& fds_entry = fds_info[i]; pfd.revents = 0; - if (fds_entry.unblocked_flags == Thread::FileBlocker::BlockFlags::None) + if (fds_entry.unblocked_flags == BlockFlags::None) continue; - if ((u32)fds_entry.unblocked_flags & (u32)Thread::FileBlocker::BlockFlags::Exception) { - if ((u32)fds_entry.unblocked_flags & (u32)Thread::FileBlocker::BlockFlags::ReadHangUp) + if (has_flag(fds_entry.unblocked_flags, BlockFlags::Exception)) { + if (has_flag(fds_entry.unblocked_flags, BlockFlags::ReadHangUp)) pfd.revents |= POLLRDHUP; - if ((u32)fds_entry.unblocked_flags & (u32)Thread::FileBlocker::BlockFlags::WriteError) + if (has_flag(fds_entry.unblocked_flags, BlockFlags::WriteError)) pfd.revents |= POLLERR; - if ((u32)fds_entry.unblocked_flags & (u32)Thread::FileBlocker::BlockFlags::WriteHangUp) + if (has_flag(fds_entry.unblocked_flags, BlockFlags::WriteHangUp)) pfd.revents |= POLLNVAL; } else { - if ((u32)fds_entry.unblocked_flags & (u32)Thread::FileBlocker::BlockFlags::Read) { + if (has_flag(fds_entry.unblocked_flags, BlockFlags::Read)) { VERIFY(pfd.events & POLLIN); pfd.revents |= POLLIN; } - if ((u32)fds_entry.unblocked_flags & (u32)Thread::FileBlocker::BlockFlags::ReadPriority) { + if (has_flag(fds_entry.unblocked_flags, BlockFlags::ReadPriority)) { VERIFY(pfd.events & POLLPRI); pfd.revents |= POLLPRI; } - if ((u32)fds_entry.unblocked_flags & (u32)Thread::FileBlocker::BlockFlags::Write) { + if (has_flag(fds_entry.unblocked_flags, BlockFlags::Write)) { VERIFY(pfd.events & POLLOUT); pfd.revents |= POLLOUT; } diff --git a/Kernel/Thread.h b/Kernel/Thread.h index 2fd7f2ea180..71c6e1526d4 100644 --- a/Kernel/Thread.h +++ b/Kernel/Thread.h @@ -26,6 +26,7 @@ #pragma once +#include #include #include #include @@ -1267,6 +1268,8 @@ private: void drop_thread_count(bool); }; +AK_ENUM_BITWISE_OPERATORS(Thread::FileBlocker::BlockFlags); + template inline IterationDecision Thread::for_each(Callback callback) { diff --git a/Kernel/ThreadBlockers.cpp b/Kernel/ThreadBlockers.cpp index 91f25674209..6794cdea6a1 100644 --- a/Kernel/ThreadBlockers.cpp +++ b/Kernel/ThreadBlockers.cpp @@ -265,17 +265,17 @@ const FileDescription& Thread::FileDescriptionBlocker::blocked_description() con } Thread::AcceptBlocker::AcceptBlocker(FileDescription& description, BlockFlags& unblocked_flags) - : FileDescriptionBlocker(description, (BlockFlags)((u32)BlockFlags::Accept | (u32)BlockFlags::Exception), unblocked_flags) + : FileDescriptionBlocker(description, BlockFlags::Accept | BlockFlags::Exception, unblocked_flags) { } Thread::ConnectBlocker::ConnectBlocker(FileDescription& description, BlockFlags& unblocked_flags) - : FileDescriptionBlocker(description, (BlockFlags)((u32)BlockFlags::Connect | (u32)BlockFlags::Exception), unblocked_flags) + : FileDescriptionBlocker(description, BlockFlags::Connect | BlockFlags::Exception, unblocked_flags) { } Thread::WriteBlocker::WriteBlocker(FileDescription& description, BlockFlags& unblocked_flags) - : FileDescriptionBlocker(description, (BlockFlags)((u32)BlockFlags::Write | (u32)BlockFlags::Exception), unblocked_flags) + : FileDescriptionBlocker(description, BlockFlags::Write | BlockFlags::Exception, unblocked_flags) { } @@ -295,7 +295,7 @@ auto Thread::WriteBlocker::override_timeout(const BlockTimeout& timeout) -> cons } Thread::ReadBlocker::ReadBlocker(FileDescription& description, BlockFlags& unblocked_flags) - : FileDescriptionBlocker(description, (BlockFlags)((u32)BlockFlags::Read | (u32)BlockFlags::Exception), unblocked_flags) + : FileDescriptionBlocker(description, BlockFlags::Read | BlockFlags::Exception, unblocked_flags) { }