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.

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,
+    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())
-        direction_in ? PacketID::IN : PacketID::OUT,
+        return result;
-        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;
 
     // Status TD always has toggle set to 1
     pipe.set_toggle(true);

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>>

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;
 }
 

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();

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)");
     }
 };

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);

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();
     }

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;
         }

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 (auto result = send_tcp_packet(TCPFlags::PUSH | TCPFlags::ACK, &data, data_length, &routing_decision); result.is_error())
-    if (err < 0)
+        return result;
-        return KResult((ErrnoCode)-err);
     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 (auto result = send_tcp_packet(TCPFlags::SYN); result.is_error())
-    if (err < 0)
+        return result;
-        return KResult((ErrnoCode)-err);
     m_state = State::SynSent;
     m_role = Role::Connecting;
     m_direction = Direction::Outgoing;

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;

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());
     }

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;
 }
 
 }

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)

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;