diff options
author | Andreas Kling <kling@serenityos.org> | 2021-08-14 15:15:11 +0200 |
---|---|---|
committer | Andreas Kling <kling@serenityos.org> | 2021-08-14 15:19:00 +0200 |
commit | 7676edfb9ba4d5052978968ba31ef940153ea7a9 (patch) | |
tree | c068023ed273da25fbecea848becc6848c738809 /Kernel | |
parent | d30d776ca46b12d7f4f9b5b7bdbfb247813a94af (diff) | |
download | serenity-7676edfb9ba4d5052978968ba31ef940153ea7a9.zip |
Kernel: Stop allowing implicit conversion from KResult to int
This patch removes KResult::operator int() and deals with the fallout.
This forces a lot of code to be more explicit in its handling of errors,
greatly improving readability.
Diffstat (limited to 'Kernel')
-rw-r--r-- | Kernel/Bus/USB/UHCIController.cpp | 12 | ||||
-rw-r--r-- | Kernel/FileSystem/Ext2FileSystem.cpp | 6 | ||||
-rw-r--r-- | Kernel/FileSystem/FileDescription.cpp | 7 | ||||
-rw-r--r-- | Kernel/FileSystem/VirtualFileSystem.cpp | 10 | ||||
-rw-r--r-- | Kernel/KResult.h | 13 | ||||
-rw-r--r-- | Kernel/Net/IPv4Socket.cpp | 4 | ||||
-rw-r--r-- | Kernel/Net/LocalSocket.cpp | 2 | ||||
-rw-r--r-- | Kernel/Net/NetworkTask.cpp | 25 | ||||
-rw-r--r-- | Kernel/Net/TCPSocket.cpp | 10 | ||||
-rw-r--r-- | Kernel/Process.cpp | 2 | ||||
-rw-r--r-- | Kernel/Syscall.cpp | 2 | ||||
-rw-r--r-- | Kernel/Syscalls/open.cpp | 4 | ||||
-rw-r--r-- | Kernel/Syscalls/stat.cpp | 4 | ||||
-rw-r--r-- | Kernel/Syscalls/unveil.cpp | 2 |
14 files changed, 51 insertions, 52 deletions
diff --git a/Kernel/Bus/USB/UHCIController.cpp b/Kernel/Bus/USB/UHCIController.cpp index 3a6c52d95b..7d36c3ae58 100644 --- a/Kernel/Bus/USB/UHCIController.cpp +++ b/Kernel/Bus/USB/UHCIController.cpp @@ -591,16 +591,8 @@ KResultOr<size_t> UHCIController::submit_control_transfer(Transfer& transfer) TransferDescriptor* last_data_descriptor; TransferDescriptor* data_descriptor_chain; auto buffer_address = Ptr32<u8>(transfer.buffer_physical().as_ptr() + sizeof(USBRequestData)); - auto transfer_chain_create_result = create_chain(pipe, - direction_in ? PacketID::IN : PacketID::OUT, - buffer_address, - pipe.max_packet_size(), - transfer.transfer_data_size(), - &data_descriptor_chain, - &last_data_descriptor); - - if (transfer_chain_create_result != KSuccess) - return transfer_chain_create_result; + if (auto result = create_chain(pipe, direction_in ? PacketID::IN : PacketID::OUT, buffer_address, pipe.max_packet_size(), transfer.transfer_data_size(), &data_descriptor_chain, &last_data_descriptor); result.is_error()) + return result; // Status TD always has toggle set to 1 pipe.set_toggle(true); diff --git a/Kernel/FileSystem/Ext2FileSystem.cpp b/Kernel/FileSystem/Ext2FileSystem.cpp index 9acb591a0f..9616eb04e0 100644 --- a/Kernel/FileSystem/Ext2FileSystem.cpp +++ b/Kernel/FileSystem/Ext2FileSystem.cpp @@ -1295,7 +1295,11 @@ bool Ext2FS::write_ext2_inode(InodeIndex inode, const ext2_inode& e2inode) if (!find_block_containing_inode(inode, block_index, offset)) return false; auto buffer = UserOrKernelBuffer::for_kernel_buffer(const_cast<u8*>((const u8*)&e2inode)); - return write_block(block_index, buffer, inode_size(), offset) >= 0; + if (auto result = write_block(block_index, buffer, inode_size(), offset); result.is_error()) { + // FIXME: Propagate errors. + return false; + } + return true; } auto Ext2FS::allocate_blocks(GroupIndex preferred_group_index, size_t count) -> KResultOr<Vector<BlockIndex>> diff --git a/Kernel/FileSystem/FileDescription.cpp b/Kernel/FileSystem/FileDescription.cpp index dc9e3c0028..3742a08e58 100644 --- a/Kernel/FileSystem/FileDescription.cpp +++ b/Kernel/FileSystem/FileDescription.cpp @@ -237,7 +237,7 @@ KResultOr<size_t> FileDescription::get_dir_entries(UserOrKernelBuffer& output_bu OutputMemoryStream stream { temp_buffer }; auto flush_stream_to_output_buffer = [&error, &stream, &remaining, &output_buffer]() -> bool { - if (error != 0) + if (error.is_error()) return false; if (stream.size() == 0) return true; @@ -273,13 +273,12 @@ KResultOr<size_t> FileDescription::get_dir_entries(UserOrKernelBuffer& output_bu // We should only return EFAULT when the userspace buffer is too small, // so that userspace can reliably use it as a signal to increase its // buffer size. - VERIFY(result != -EFAULT); + VERIFY(result != EFAULT); return result; } - if (error) { + if (error.is_error()) return error; - } return size - remaining; } diff --git a/Kernel/FileSystem/VirtualFileSystem.cpp b/Kernel/FileSystem/VirtualFileSystem.cpp index df338469ce..81fb54a9d8 100644 --- a/Kernel/FileSystem/VirtualFileSystem.cpp +++ b/Kernel/FileSystem/VirtualFileSystem.cpp @@ -222,7 +222,7 @@ KResultOr<NonnullRefPtr<FileDescription>> VirtualFileSystem::open(StringView pat if (custody_or_error.is_error()) { // NOTE: ENOENT with a non-null parent custody signals us that the immediate parent // of the file exists, but the file itself does not. - if ((options & O_CREAT) && custody_or_error.error() == -ENOENT && parent_custody) + if ((options & O_CREAT) && custody_or_error.error() == ENOENT && parent_custody) return create(path, options, mode, *parent_custody, move(owner)); return custody_or_error.error(); } @@ -326,7 +326,7 @@ KResult VirtualFileSystem::mknod(StringView path, mode_t mode, dev_t dev, Custod return EEXIST; if (!parent_custody) return ENOENT; - if (existing_file_or_error.error() != -ENOENT) + if (existing_file_or_error.error() != ENOENT) return existing_file_or_error.error(); auto& parent_inode = parent_custody->inode(); auto current_process = Process::current(); @@ -401,7 +401,7 @@ KResult VirtualFileSystem::mkdir(StringView path, mode_t mode, Custody& base) else if (!parent_custody) return result.error(); // NOTE: If resolve_path fails with a non-null parent custody, the error should be ENOENT. - VERIFY(result.error() == -ENOENT); + VERIFY(result.error() == ENOENT); auto& parent_inode = parent_custody->inode(); auto current_process = Process::current(); @@ -491,7 +491,7 @@ KResult VirtualFileSystem::rename(StringView old_path, StringView new_path, Cust RefPtr<Custody> new_parent_custody; auto new_custody_or_error = resolve_path(new_path, base, &new_parent_custody); if (new_custody_or_error.is_error()) { - if (new_custody_or_error.error() != -ENOENT || !new_parent_custody) + if (new_custody_or_error.error() != ENOENT || !new_parent_custody) return new_custody_or_error.error(); } @@ -720,7 +720,7 @@ KResult VirtualFileSystem::symlink(StringView target, StringView linkpath, Custo return EEXIST; if (!parent_custody) return ENOENT; - if (existing_custody_or_error.error() != -ENOENT) + if (existing_custody_or_error.is_error() && existing_custody_or_error.error() != ENOENT) return existing_custody_or_error.error(); auto& parent_inode = parent_custody->inode(); auto current_process = Process::current(); diff --git a/Kernel/KResult.h b/Kernel/KResult.h index 05ae90ba25..5b70fd7681 100644 --- a/Kernel/KResult.h +++ b/Kernel/KResult.h @@ -28,12 +28,17 @@ public: : m_error(0) { } - operator int() const { return m_error; } [[nodiscard]] int error() const { return m_error; } [[nodiscard]] bool is_success() const { return m_error == 0; } [[nodiscard]] bool is_error() const { return !is_success(); } + bool operator==(ErrnoCode error) const { return is_error() && m_error == -error; } + bool operator!=(ErrnoCode error) const { return !is_error() || m_error != -error; } + + bool operator!=(KSuccessTag) const { return is_error(); } + bool operator==(KSuccessTag) const { return !is_error(); } + private: template<typename T> friend class KResultOr; @@ -167,9 +172,11 @@ private: } template<> -struct AK::Formatter<Kernel::KResult> : Formatter<int> { +struct AK::Formatter<Kernel::KResult> : Formatter<FormatString> { void format(FormatBuilder& builder, Kernel::KResult value) { - return Formatter<int>::format(builder, value); + if (value.is_error()) + return AK::Formatter<FormatString>::format(builder, "KResult({})", value.error()); + return AK::Formatter<FormatString>::format(builder, "KResult(success)"); } }; diff --git a/Kernel/Net/IPv4Socket.cpp b/Kernel/Net/IPv4Socket.cpp index ad2994bea1..b64bf47547 100644 --- a/Kernel/Net/IPv4Socket.cpp +++ b/Kernel/Net/IPv4Socket.cpp @@ -137,7 +137,7 @@ KResult IPv4Socket::listen(size_t backlog) { MutexLocker locker(lock()); auto result = allocate_local_port_if_needed(); - if (result.error_or_port.is_error() && result.error_or_port.error() != -ENOPROTOOPT) + if (result.error_or_port.is_error() && result.error_or_port.error() != ENOPROTOOPT) return result.error_or_port.error(); set_backlog(backlog); @@ -231,7 +231,7 @@ KResultOr<size_t> IPv4Socket::sendto(FileDescription&, const UserOrKernelBuffer& if (m_local_address.to_u32() == 0) m_local_address = routing_decision.adapter->ipv4_address(); - if (auto result = allocate_local_port_if_needed(); result.error_or_port.is_error() && result.error_or_port.error() != -ENOPROTOOPT) + if (auto result = allocate_local_port_if_needed(); result.error_or_port.is_error() && result.error_or_port.error() != ENOPROTOOPT) return result.error_or_port.error(); dbgln_if(IPV4_SOCKET_DEBUG, "sendto: destination={}:{}", m_peer_address, m_peer_port); diff --git a/Kernel/Net/LocalSocket.cpp b/Kernel/Net/LocalSocket.cpp index d6be8a6770..7d23684d77 100644 --- a/Kernel/Net/LocalSocket.cpp +++ b/Kernel/Net/LocalSocket.cpp @@ -139,7 +139,7 @@ KResult LocalSocket::bind(Userspace<const sockaddr*> user_address, socklen_t add UidAndGid owner { m_prebind_uid, m_prebind_gid }; auto result = VirtualFileSystem::the().open(path, O_CREAT | O_EXCL | O_NOFOLLOW_NOERROR, mode, Process::current()->current_directory(), owner); if (result.is_error()) { - if (result.error() == -EEXIST) + if (result.error() == EEXIST) return set_so_error(EADDRINUSE); return result.error(); } diff --git a/Kernel/Net/NetworkTask.cpp b/Kernel/Net/NetworkTask.cpp index 6adc79953d..620e52a006 100644 --- a/Kernel/Net/NetworkTask.cpp +++ b/Kernel/Net/NetworkTask.cpp @@ -423,7 +423,6 @@ void handle_tcp(IPv4Packet const& ipv4_packet, Time const& packet_timestamp) socket->receive_tcp_packet(tcp_packet, ipv4_packet.payload_size()); - [[maybe_unused]] int unused_rc {}; switch (socket->state()) { case TCPSocket::State::Closed: dbgln("handle_tcp: unexpected flags in Closed state"); @@ -431,7 +430,7 @@ void handle_tcp(IPv4Packet const& ipv4_packet, Time const& packet_timestamp) return; case TCPSocket::State::TimeWait: dbgln("handle_tcp: unexpected flags in TimeWait state"); - unused_rc = socket->send_tcp_packet(TCPFlags::RST); + (void)socket->send_tcp_packet(TCPFlags::RST); socket->set_state(TCPSocket::State::Closed); return; case TCPSocket::State::Listen: @@ -462,12 +461,12 @@ void handle_tcp(IPv4Packet const& ipv4_packet, Time const& packet_timestamp) switch (tcp_packet.flags()) { case TCPFlags::SYN: socket->set_ack_number(tcp_packet.sequence_number() + payload_size + 1); - unused_rc = socket->send_ack(true); + (void)socket->send_ack(true); socket->set_state(TCPSocket::State::SynReceived); return; case TCPFlags::ACK | TCPFlags::SYN: socket->set_ack_number(tcp_packet.sequence_number() + payload_size + 1); - unused_rc = socket->send_ack(true); + (void)socket->send_ack(true); socket->set_state(TCPSocket::State::Established); socket->set_setup_state(Socket::SetupState::Completed); socket->set_connected(true); @@ -486,7 +485,7 @@ void handle_tcp(IPv4Packet const& ipv4_packet, Time const& packet_timestamp) return; default: dbgln("handle_tcp: unexpected flags in SynSent state ({:x})", tcp_packet.flags()); - unused_rc = socket->send_tcp_packet(TCPFlags::RST); + (void)socket->send_tcp_packet(TCPFlags::RST); socket->set_state(TCPSocket::State::Closed); socket->set_error(TCPSocket::Error::UnexpectedFlagsDuringConnect); socket->set_setup_state(Socket::SetupState::Completed); @@ -501,7 +500,7 @@ void handle_tcp(IPv4Packet const& ipv4_packet, Time const& packet_timestamp) case TCPSocket::Direction::Incoming: if (!socket->has_originator()) { dbgln("handle_tcp: connection doesn't have an originating socket; maybe it went away?"); - unused_rc = socket->send_tcp_packet(TCPFlags::RST); + (void)socket->send_tcp_packet(TCPFlags::RST); socket->set_state(TCPSocket::State::Closed); return; } @@ -517,7 +516,7 @@ void handle_tcp(IPv4Packet const& ipv4_packet, Time const& packet_timestamp) return; default: dbgln("handle_tcp: got ACK in SynReceived state but direction is invalid ({})", TCPSocket::to_string(socket->direction())); - unused_rc = socket->send_tcp_packet(TCPFlags::RST); + (void)socket->send_tcp_packet(TCPFlags::RST); socket->set_state(TCPSocket::State::Closed); return; } @@ -528,7 +527,7 @@ void handle_tcp(IPv4Packet const& ipv4_packet, Time const& packet_timestamp) return; default: dbgln("handle_tcp: unexpected flags in SynReceived state ({:x})", tcp_packet.flags()); - unused_rc = socket->send_tcp_packet(TCPFlags::RST); + (void)socket->send_tcp_packet(TCPFlags::RST); socket->set_state(TCPSocket::State::Closed); return; } @@ -536,7 +535,7 @@ void handle_tcp(IPv4Packet const& ipv4_packet, Time const& packet_timestamp) switch (tcp_packet.flags()) { default: dbgln("handle_tcp: unexpected flags in CloseWait state ({:x})", tcp_packet.flags()); - unused_rc = socket->send_tcp_packet(TCPFlags::RST); + (void)socket->send_tcp_packet(TCPFlags::RST); socket->set_state(TCPSocket::State::Closed); return; } @@ -548,7 +547,7 @@ void handle_tcp(IPv4Packet const& ipv4_packet, Time const& packet_timestamp) return; default: dbgln("handle_tcp: unexpected flags in LastAck state ({:x})", tcp_packet.flags()); - unused_rc = socket->send_tcp_packet(TCPFlags::RST); + (void)socket->send_tcp_packet(TCPFlags::RST); socket->set_state(TCPSocket::State::Closed); return; } @@ -564,7 +563,7 @@ void handle_tcp(IPv4Packet const& ipv4_packet, Time const& packet_timestamp) return; default: dbgln("handle_tcp: unexpected flags in FinWait1 state ({:x})", tcp_packet.flags()); - unused_rc = socket->send_tcp_packet(TCPFlags::RST); + (void)socket->send_tcp_packet(TCPFlags::RST); socket->set_state(TCPSocket::State::Closed); return; } @@ -579,7 +578,7 @@ void handle_tcp(IPv4Packet const& ipv4_packet, Time const& packet_timestamp) return; default: dbgln("handle_tcp: unexpected flags in FinWait2 state ({:x})", tcp_packet.flags()); - unused_rc = socket->send_tcp_packet(TCPFlags::RST); + (void)socket->send_tcp_packet(TCPFlags::RST); socket->set_state(TCPSocket::State::Closed); return; } @@ -591,7 +590,7 @@ void handle_tcp(IPv4Packet const& ipv4_packet, Time const& packet_timestamp) return; default: dbgln("handle_tcp: unexpected flags in Closing state ({:x})", tcp_packet.flags()); - unused_rc = socket->send_tcp_packet(TCPFlags::RST); + (void)socket->send_tcp_packet(TCPFlags::RST); socket->set_state(TCPSocket::State::Closed); return; } diff --git a/Kernel/Net/TCPSocket.cpp b/Kernel/Net/TCPSocket.cpp index 4cae65183c..b55982b9a4 100644 --- a/Kernel/Net/TCPSocket.cpp +++ b/Kernel/Net/TCPSocket.cpp @@ -184,9 +184,8 @@ KResultOr<size_t> TCPSocket::protocol_send(const UserOrKernelBuffer& data, size_ return set_so_error(EHOSTUNREACH); size_t mss = routing_decision.adapter->mtu() - sizeof(IPv4Packet) - sizeof(TCPPacket); data_length = min(data_length, mss); - int err = send_tcp_packet(TCPFlags::PUSH | TCPFlags::ACK, &data, data_length, &routing_decision); - if (err < 0) - return KResult((ErrnoCode)-err); + if (auto result = send_tcp_packet(TCPFlags::PUSH | TCPFlags::ACK, &data, data_length, &routing_decision); result.is_error()) + return result; return data_length; } @@ -414,9 +413,8 @@ KResult TCPSocket::protocol_connect(FileDescription& description, ShouldBlock sh m_ack_number = 0; set_setup_state(SetupState::InProgress); - int err = send_tcp_packet(TCPFlags::SYN); - if (err < 0) - return KResult((ErrnoCode)-err); + if (auto result = send_tcp_packet(TCPFlags::SYN); result.is_error()) + return result; m_state = State::SynSent; m_role = Role::Connecting; m_direction = Direction::Outgoing; diff --git a/Kernel/Process.cpp b/Kernel/Process.cpp index 4eb4d69c89..14e6c2bd64 100644 --- a/Kernel/Process.cpp +++ b/Kernel/Process.cpp @@ -174,7 +174,7 @@ RefPtr<Process> Process::create_user_process(RefPtr<Thread>& first_thread, const setup_description(1); setup_description(2); - error = process->exec(path, move(arguments), move(environment)); + error = process->exec(path, move(arguments), move(environment)).error(); if (error != 0) { dbgln("Failed to exec {}: {}", path, error); first_thread = nullptr; diff --git a/Kernel/Syscall.cpp b/Kernel/Syscall.cpp index 6c71c0b94e..1b89ff35dd 100644 --- a/Kernel/Syscall.cpp +++ b/Kernel/Syscall.cpp @@ -216,7 +216,7 @@ NEVER_INLINE void syscall_handler(TrapFrame* trap) auto result = Syscall::handle(regs, function, arg1, arg2, arg3, arg4); if (result.is_error()) { - regs.set_return_reg(result.error()); + regs.set_return_reg(result.error().error()); } else { regs.set_return_reg(result.value()); } diff --git a/Kernel/Syscalls/open.cpp b/Kernel/Syscalls/open.cpp index 209396ca97..77f2039669 100644 --- a/Kernel/Syscalls/open.cpp +++ b/Kernel/Syscalls/open.cpp @@ -84,9 +84,9 @@ KResultOr<FlatPtr> Process::sys$close(int fd) dbgln_if(IO_DEBUG, "sys$close({}) {}", fd, description.ptr()); if (!description) return EBADF; - int rc = description->close(); + auto result = description->close(); m_fds[fd] = {}; - return rc; + return result; } } diff --git a/Kernel/Syscalls/stat.cpp b/Kernel/Syscalls/stat.cpp index 7d42d49a2b..ac31ec9ce0 100644 --- a/Kernel/Syscalls/stat.cpp +++ b/Kernel/Syscalls/stat.cpp @@ -19,10 +19,10 @@ KResultOr<FlatPtr> Process::sys$fstat(int fd, Userspace<stat*> user_statbuf) if (!description) return EBADF; stat buffer = {}; - int rc = description->stat(buffer); + auto result = description->stat(buffer); if (!copy_to_user(user_statbuf, &buffer)) return EFAULT; - return rc; + return result; } KResultOr<FlatPtr> Process::sys$stat(Userspace<const Syscall::SC_stat_params*> user_params) diff --git a/Kernel/Syscalls/unveil.cpp b/Kernel/Syscalls/unveil.cpp index 7c7bfb7b95..710011a8e4 100644 --- a/Kernel/Syscalls/unveil.cpp +++ b/Kernel/Syscalls/unveil.cpp @@ -97,7 +97,7 @@ KResultOr<FlatPtr> Process::sys$unveil(Userspace<const Syscall::SC_unveil_params new_unveiled_path = custody_or_error.value()->try_create_absolute_path(); if (!new_unveiled_path) return ENOMEM; - } else if (custody_or_error.error() == -ENOENT && parent_custody && (new_permissions & UnveilAccess::CreateOrRemove)) { + } else if (custody_or_error.error() == ENOENT && parent_custody && (new_permissions & UnveilAccess::CreateOrRemove)) { auto parent_custody_path = parent_custody->try_create_absolute_path(); if (!parent_custody_path) return ENOMEM; |