summaryrefslogtreecommitdiff
path: root/Kernel
diff options
context:
space:
mode:
authorAndreas Kling <kling@serenityos.org>2021-08-14 15:15:11 +0200
committerAndreas Kling <kling@serenityos.org>2021-08-14 15:19:00 +0200
commit7676edfb9ba4d5052978968ba31ef940153ea7a9 (patch)
treec068023ed273da25fbecea848becc6848c738809 /Kernel
parentd30d776ca46b12d7f4f9b5b7bdbfb247813a94af (diff)
downloadserenity-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.cpp12
-rw-r--r--Kernel/FileSystem/Ext2FileSystem.cpp6
-rw-r--r--Kernel/FileSystem/FileDescription.cpp7
-rw-r--r--Kernel/FileSystem/VirtualFileSystem.cpp10
-rw-r--r--Kernel/KResult.h13
-rw-r--r--Kernel/Net/IPv4Socket.cpp4
-rw-r--r--Kernel/Net/LocalSocket.cpp2
-rw-r--r--Kernel/Net/NetworkTask.cpp25
-rw-r--r--Kernel/Net/TCPSocket.cpp10
-rw-r--r--Kernel/Process.cpp2
-rw-r--r--Kernel/Syscall.cpp2
-rw-r--r--Kernel/Syscalls/open.cpp4
-rw-r--r--Kernel/Syscalls/stat.cpp4
-rw-r--r--Kernel/Syscalls/unveil.cpp2
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;