diff --git a/kernel/src/sys/file.cpp b/kernel/src/sys/file.cpp index 8e3b8dee..9c7979bb 100644 --- a/kernel/src/sys/file.cpp +++ b/kernel/src/sys/file.cpp @@ -15,17 +15,13 @@ Result sys_read(Registers*, SyscallArgs args) if (!MemoryManager::validate_user_write(buf, size)) return err(EFAULT); - if (fd < 0 || fd >= FD_MAX) return err(EBADF); - Thread* current = Scheduler::current(); - Option& descriptor = current->fd_table->fds[fd]; + auto& descriptor = *TRY(current->resolve_fd(fd)); - if (!descriptor.has_value()) return err(EBADF); + usize nread = TRY(descriptor.inode->read(buf, descriptor.offset, size)); - usize nread = TRY(descriptor->inode->read(buf, descriptor->offset, size)); - - descriptor->offset += nread; + descriptor.offset += nread; return nread; } @@ -38,17 +34,13 @@ Result sys_write(Registers*, SyscallArgs args) if (!MemoryManager::validate_user_read(buf, size)) return err(EFAULT); - if (fd < 0 || fd >= FD_MAX) return err(EBADF); - Thread* current = Scheduler::current(); - Option& descriptor = current->fd_table->fds[fd]; + auto& descriptor = *TRY(current->resolve_fd(fd)); - if (!descriptor.has_value()) return err(EBADF); + usize nwritten = TRY(descriptor.inode->write(buf, descriptor.offset, size)); - usize nwritten = TRY(descriptor->inode->write(buf, descriptor->offset, size)); - - descriptor->offset += nwritten; + descriptor.offset += nwritten; return nwritten; } @@ -59,27 +51,23 @@ Result sys_lseek(Registers*, SyscallArgs args) off_t offset = (long)args[1]; int whence = (int)args[2]; - if (fd < 0 || fd >= FD_MAX) return err(EBADF); - Thread* current = Scheduler::current(); - Option& descriptor = current->fd_table->fds[fd]; - - if (!descriptor.has_value()) return err(EBADF); + auto& descriptor = *TRY(current->resolve_fd(fd)); off_t new_offset; switch (whence) { case SEEK_SET: new_offset = offset; break; - case SEEK_CUR: new_offset = TRY(safe_add((long)descriptor->offset, offset)); break; + case SEEK_CUR: new_offset = TRY(safe_add((long)descriptor.offset, offset)); break; case SEEK_END: todo(); default: return err(EINVAL); } if (new_offset < 0) return err(EINVAL); - descriptor->offset = (usize)new_offset; + descriptor.offset = (usize)new_offset; return (u64)new_offset; } diff --git a/kernel/src/sys/open.cpp b/kernel/src/sys/open.cpp index d2decfde..f5cb7a0c 100644 --- a/kernel/src/sys/open.cpp +++ b/kernel/src/sys/open.cpp @@ -20,7 +20,7 @@ Result sys_open(Registers*, SyscallArgs args) int fd = TRY(current->allocate_fd(0)); - current->fd_table->fds[fd] = FileDescriptor { inode, 0, flags }; + current->fd_table[fd] = FileDescriptor { inode, 0, flags }; kinfoln("open: allocated file descriptor %d for inode %zu", fd, inode->inode_number()); @@ -34,7 +34,7 @@ Result sys_close(Registers*, SyscallArgs args) Thread* current = Scheduler::current(); - Option& descriptor = current->fd_table->fds[fd]; + Option& descriptor = current->fd_table[fd]; if (!descriptor.has_value()) return err(EBADF); diff --git a/kernel/src/thread/Scheduler.cpp b/kernel/src/thread/Scheduler.cpp index a733c579..0f1eb7cb 100644 --- a/kernel/src/thread/Scheduler.cpp +++ b/kernel/src/thread/Scheduler.cpp @@ -151,8 +151,6 @@ namespace Scheduler guard.deactivate(); directory_guard.deactivate(); - thread->fd_table = FileDescriptorTable {}; - kinfoln("Created userspace thread: id %lu with ip %#.16lx and sp %#.16lx (ksp %#lx)", thread->id, thread->ip(), thread->sp(), thread->kernel_stack.top()); diff --git a/kernel/src/thread/Thread.cpp b/kernel/src/thread/Thread.cpp index 04fc2252..f01c70c0 100644 --- a/kernel/src/thread/Thread.cpp +++ b/kernel/src/thread/Thread.cpp @@ -27,8 +27,19 @@ Result Thread::allocate_fd(int min) { // FIXME: Possible race condition if multiple threads share a FileDescriptorTable? Let's not worry about it for // now, we're still a long way away from reaching that point. - if (!fd_table->fds[i].has_value()) { return i; } + if (!fd_table[i].has_value()) { return i; } } return err(EMFILE); } + +Result Thread::resolve_fd(int fd) +{ + if (fd < 0 || fd >= FD_MAX) return err(EBADF); + + Option& maybe_descriptor = fd_table[fd]; + + if (!maybe_descriptor.has_value()) return err(EBADF); + + return maybe_descriptor.value_ptr(); +} diff --git a/kernel/src/thread/Thread.h b/kernel/src/thread/Thread.h index ca1b5aa5..58787351 100644 --- a/kernel/src/thread/Thread.h +++ b/kernel/src/thread/Thread.h @@ -31,11 +31,6 @@ struct FileDescriptor static constexpr int FD_MAX = 64; -struct FileDescriptorTable -{ - Option fds[FD_MAX] = {}; -}; - struct Thread : public LinkedListNode { Registers regs; @@ -53,9 +48,10 @@ struct Thread : public LinkedListNode Stack kernel_stack; OwnedPtr vm_allocator; - Option fd_table; + Option fd_table[FD_MAX] = {}; Result allocate_fd(int min); + Result resolve_fd(int fd); FPData fp_data; diff --git a/libluna/include/luna/Option.h b/libluna/include/luna/Option.h index 40307203..20c2a9cb 100644 --- a/libluna/include/luna/Option.h +++ b/libluna/include/luna/Option.h @@ -116,6 +116,18 @@ template class Option return &m_storage.fetch_reference(); } + T& operator*() + { + expect(has_value(), "Option::operator* called on an empty Option"); + return m_storage.fetch_reference(); + } + + T* value_ptr(SourceLocation caller = SourceLocation::current()) + { + expect_at(has_value(), caller, "Option::value_ptr called on an empty Option"); + return &m_storage.fetch_reference(); + } + ~Option() { if (has_value()) m_storage.destroy();