From ff2e2937a5d0cd66f9d6020d9f59cdec06f8e68a Mon Sep 17 00:00:00 2001 From: Bananymous Date: Tue, 9 May 2023 20:31:22 +0300 Subject: [PATCH] Kernel: Remove offset from OpenFileDescriptor This is now handled on the libc side. There might be reasons to have it in kernel side, but for simplicity's sake I'm moving it to libc for now :) --- LibELF/LibELF/ELF.cpp | 2 +- kernel/arch/x86_64/interrupts.S | 4 +++ kernel/include/kernel/Process.h | 16 +++++----- kernel/include/kernel/Syscall.h | 13 ++++---- kernel/kernel/Font.cpp | 2 +- kernel/kernel/Process.cpp | 54 ++++++--------------------------- kernel/kernel/Shell.cpp | 18 ++++++----- kernel/kernel/Syscall.cpp | 33 ++++++++------------ kernel/kernel/Terminal/TTY.cpp | 2 +- libc/stdio.cpp | 24 ++++++--------- libc/unistd.cpp | 19 +++++------- userspace/test.c | 45 ++++++++++++--------------- 12 files changed, 90 insertions(+), 142 deletions(-) diff --git a/LibELF/LibELF/ELF.cpp b/LibELF/LibELF/ELF.cpp index 928885ddf9..27998a9c71 100644 --- a/LibELF/LibELF/ELF.cpp +++ b/LibELF/LibELF/ELF.cpp @@ -25,7 +25,7 @@ namespace LibELF TRY(data.resize(st.st_size)); - TRY(Kernel::Process::current().read(fd, data.data(), data.size())); + TRY(Kernel::Process::current().read(fd, data.data(), 0, data.size())); elf = new ELF(BAN::move(data)); ASSERT(elf); diff --git a/kernel/arch/x86_64/interrupts.S b/kernel/arch/x86_64/interrupts.S index d8dd93c58c..ad6aa9719c 100644 --- a/kernel/arch/x86_64/interrupts.S +++ b/kernel/arch/x86_64/interrupts.S @@ -153,9 +153,13 @@ irq 13 irq 14 irq 15 +// arguments in RAX, RBX, RCX, RDX, RSI, RDI +// System V ABI: RDI, RSI, RDX, RCX, R8, R9 .global syscall_asm syscall_asm: pushaq + movq %rsi, %r8 + movq %rdi, %r9 movq %rax, %rdi movq %rbx, %rsi xchgq %rcx, %rdx diff --git a/kernel/include/kernel/Process.h b/kernel/include/kernel/Process.h index 6114aff98b..6f85dbe556 100644 --- a/kernel/include/kernel/Process.h +++ b/kernel/include/kernel/Process.h @@ -41,16 +41,15 @@ namespace Kernel pid_t pid() const { return m_pid; } BAN::ErrorOr open(BAN::StringView, int); - BAN::ErrorOr close(int); - BAN::ErrorOr read(int, void*, size_t); - BAN::ErrorOr write(int, const void*, size_t); - BAN::ErrorOr creat(BAN::StringView, mode_t); - BAN::ErrorOr seek(int, size_t); + BAN::ErrorOr close(int fd); + BAN::ErrorOr read(int fd, void* buffer, size_t offset, size_t count); + BAN::ErrorOr write(int fd, const void* buffer, size_t offset, size_t count); + BAN::ErrorOr creat(BAN::StringView name, mode_t); - BAN::ErrorOr fstat(int, stat*); - BAN::ErrorOr stat(BAN::StringView, stat*); + BAN::ErrorOr fstat(int fd, stat*); + BAN::ErrorOr stat(BAN::StringView path, stat*); - BAN::ErrorOr mount(BAN::StringView, BAN::StringView); + BAN::ErrorOr mount(BAN::StringView source, BAN::StringView target); BAN::ErrorOr> read_directory_entries(int); @@ -78,7 +77,6 @@ namespace Kernel { BAN::RefPtr inode; BAN::String path; - size_t offset = 0; uint8_t flags = 0; }; diff --git a/kernel/include/kernel/Syscall.h b/kernel/include/kernel/Syscall.h index 0d3574ce89..a3e52e14b7 100644 --- a/kernel/include/kernel/Syscall.h +++ b/kernel/include/kernel/Syscall.h @@ -5,21 +5,20 @@ #define SYS_WRITE 3 #define SYS_TERMID 4 #define SYS_CLOSE 5 -#define SYS_SEEK 6 -#define SYS_OPEN 7 -#define SYS_ALLOC 8 -#define SYS_FREE 9 +#define SYS_OPEN 6 +#define SYS_ALLOC 7 +#define SYS_FREE 8 +#include #include namespace Kernel { - template - inline long syscall(int syscall, T1 arg1 = nullptr, T2 arg2 = nullptr, T3 arg3 = nullptr) + ALWAYS_INLINE long syscall(int syscall, uintptr_t arg1 = 0, uintptr_t arg2 = 0, uintptr_t arg3 = 0, uintptr_t arg4 = 0, uintptr_t arg5 = 0) { long ret; - asm volatile("int $0x80" : "=a"(ret) : "a"(syscall), "b"((uintptr_t)arg1), "c"((uintptr_t)arg2), "d"((uintptr_t)arg3) : "memory"); + asm volatile("int $0x80" : "=a"(ret) : "a"(syscall), "b"((uintptr_t)arg1), "c"((uintptr_t)arg2), "d"((uintptr_t)arg3), "S"((uintptr_t)arg4), "D"((uintptr_t)arg5) : "memory"); return ret; } diff --git a/kernel/kernel/Font.cpp b/kernel/kernel/Font.cpp index 160e5aa76c..7000d7eeaf 100644 --- a/kernel/kernel/Font.cpp +++ b/kernel/kernel/Font.cpp @@ -45,7 +45,7 @@ namespace Kernel BAN::Vector file_data; TRY(file_data.resize(st.st_size)); - TRY(Process::current().read(fd, file_data.data(), st.st_size)); + TRY(Process::current().read(fd, file_data.data(), 0, st.st_size)); if (file_data.size() < 4) return BAN::Error::from_error_code(ErrorCode::Font_FileTooSmall); diff --git a/kernel/kernel/Process.cpp b/kernel/kernel/Process.cpp index 5cc387aa85..a3c9b0d80d 100644 --- a/kernel/kernel/Process.cpp +++ b/kernel/kernel/Process.cpp @@ -202,7 +202,6 @@ namespace Kernel auto& open_file_description = m_open_files[fd]; open_file_description.inode = file.inode; open_file_description.path = BAN::move(file.canonical_path); - open_file_description.offset = 0; open_file_description.flags = flags; return fd; @@ -217,7 +216,7 @@ namespace Kernel return {}; } - BAN::ErrorOr Process::read(int fd, void* buffer, size_t count) + BAN::ErrorOr Process::read(int fd, void* buffer, size_t offset, size_t count) { OpenFileDescription open_fd_copy; @@ -229,18 +228,10 @@ namespace Kernel if (!(open_fd_copy.flags & O_RDONLY)) return BAN::Error::from_errno(EBADF); - size_t n_read = TRY(open_fd_copy.inode->read(open_fd_copy.offset, buffer, count)); - open_fd_copy.offset += n_read; - - m_lock.lock(); - MUST(validate_fd(fd)); - open_file_description(fd) = open_fd_copy; - m_lock.unlock(); - - return n_read; + return TRY(open_fd_copy.inode->read(offset, buffer, count)); } - BAN::ErrorOr Process::write(int fd, const void* buffer, size_t count) + BAN::ErrorOr Process::write(int fd, const void* buffer, size_t offset, size_t count) { OpenFileDescription open_fd_copy; @@ -252,16 +243,7 @@ namespace Kernel if (!(open_fd_copy.flags & O_WRONLY)) return BAN::Error::from_errno(EBADF); - size_t n_written = TRY(open_fd_copy.inode->write(open_fd_copy.offset, buffer, count)); - open_fd_copy.offset += n_written; - - { - LockGuard _(m_lock); - MUST(validate_fd(fd)); - open_file_description(fd) = open_fd_copy; - } - - return n_written; + return TRY(open_fd_copy.inode->write(offset, buffer, count)); } BAN::ErrorOr Process::creat(BAN::StringView path, mode_t mode) @@ -282,11 +264,11 @@ namespace Kernel return {}; } - BAN::ErrorOr Process::mount(BAN::StringView partition, BAN::StringView path) + BAN::ErrorOr Process::mount(BAN::StringView source, BAN::StringView target) { - auto absolute_partition = TRY(absolute_path_of(partition)); - auto absolute_path = TRY(absolute_path_of(path)); - TRY(VirtualFileSystem::get().mount(absolute_partition, absolute_path)); + auto absolute_source = TRY(absolute_path_of(source)); + auto absolute_target = TRY(absolute_path_of(target)); + TRY(VirtualFileSystem::get().mount(absolute_source, absolute_target)); return {}; } @@ -325,15 +307,7 @@ namespace Kernel return ret; } - BAN::ErrorOr Process::seek(int fd, size_t offset) - { - LockGuard _(m_lock); - TRY(validate_fd(fd)); - auto& open_fd = open_file_description(fd); - open_fd.offset = offset; - return {}; - } - + // FIXME: This whole API has to be rewritten BAN::ErrorOr> Process::read_directory_entries(int fd) { OpenFileDescription open_fd_copy; @@ -344,15 +318,7 @@ namespace Kernel open_fd_copy = open_file_description(fd); } - auto result = TRY(open_fd_copy.inode->read_directory_entries(open_fd_copy.offset)); - open_fd_copy.offset++; - - m_lock.lock(); - MUST(validate_fd(fd)); - open_file_description(fd) = open_fd_copy; - m_lock.unlock(); - - return result; + return TRY(open_fd_copy.inode->read_directory_entries(0)); } BAN::ErrorOr Process::working_directory() const diff --git a/kernel/kernel/Shell.cpp b/kernel/kernel/Shell.cpp index 00a523ff56..60a6d8e323 100644 --- a/kernel/kernel/Shell.cpp +++ b/kernel/kernel/Shell.cpp @@ -22,14 +22,14 @@ namespace Kernel static void TTY_PRINT(Args&&... args) { BAN::String message = BAN::String::formatted(BAN::forward(args)...); - MUST(Process::current().write(STDOUT_FILENO, message.data(), message.size())); + MUST(Process::current().write(STDOUT_FILENO, message.data(), 0, message.size())); } template static void TTY_PRINTLN(Args&&... args) { TTY_PRINT(BAN::forward(args)...); - MUST(Process::current().write(STDOUT_FILENO, "\n", 1)); + MUST(Process::current().write(STDOUT_FILENO, "\n", 0, 1)); } static auto s_default_prompt = "\\[\e[32m\\]user\\[\e[m\\]:\\[\e[34m\\]\\w\\[\e[m\\]# "sv; @@ -112,7 +112,7 @@ namespace Kernel void Shell::run() { - auto getch = [this] { uint8_t ch; MUST(Process::current().read(STDIN_FILENO, &ch, 1)); return ch; }; + auto getch = [this] { uint8_t ch; MUST(Process::current().read(STDIN_FILENO, &ch, 0, 1)); return ch; }; MUST(m_buffer.push_back(""sv)); @@ -130,7 +130,7 @@ namespace Kernel while ((current.back() & 0xC0) == 0x80) current.pop_back(); current.pop_back(); - MUST(Process::current().write(STDOUT_FILENO, "\b \b", 3)); + MUST(Process::current().write(STDOUT_FILENO, "\b \b", 0, 3)); } continue; } @@ -171,7 +171,7 @@ namespace Kernel continue; } - MUST(Process::current().write(STDOUT_FILENO, &ch, 1)); + MUST(Process::current().write(STDOUT_FILENO, &ch, 0, 1)); if (ch != '\n') { @@ -556,8 +556,12 @@ argument_done: BAN::ScopeGuard buffer_guard([buffer] { delete[] buffer; }); ASSERT(buffer); - while (size_t n_read = TRY(Process::current().read(fd, buffer, buffer_size))) + size_t offset = 0; + while (size_t n_read = TRY(Process::current().read(fd, buffer, offset, buffer_size))) + { TTY_PRINT("{}", BAN::StringView(buffer, n_read)); + offset += n_read; + } TTY_PRINTLN(""); } else if (arguments.front() == "stat") @@ -635,7 +639,7 @@ argument_done: while (true) { - size_t n_read = TRY(Process::current().read(fd, buffer, buffer_size)); + size_t n_read = TRY(Process::current().read(fd, buffer, total_read, buffer_size)); if (n_read == 0) break; for (size_t j = 0; j < n_read; j++) diff --git a/kernel/kernel/Syscall.cpp b/kernel/kernel/Syscall.cpp index 1b56dbd8a1..160058ed5e 100644 --- a/kernel/kernel/Syscall.cpp +++ b/kernel/kernel/Syscall.cpp @@ -10,17 +10,17 @@ namespace Kernel Process::current().exit(); } - long sys_read(int fd, void* buffer, size_t size) + long sys_read(int fd, void* buffer, size_t offset, size_t size) { - auto res = Process::current().read(fd, buffer, size); + auto res = Process::current().read(fd, buffer, offset, size); if (res.is_error()) return -res.error().get_error_code(); return res.value(); } - long sys_write(int fd, const void* buffer, size_t size) + long sys_write(int fd, const void* buffer, size_t offset, size_t size) { - auto res = Process::current().write(fd, buffer, size); + auto res = Process::current().write(fd, buffer, offset, size); if (res.is_error()) return -res.error().get_error_code(); return res.value(); @@ -34,14 +34,6 @@ namespace Kernel return 0; } - int sys_seek(int fd, long offset) - { - auto res = Process::current().seek(fd, offset); - if (res.is_error()) - return -res.error().get_error_code(); - return 0; - } - void sys_termid(char* buffer) { Process::current().termid(buffer); @@ -68,12 +60,14 @@ namespace Kernel Process::current().free(ptr); } - extern "C" long cpp_syscall_handler(int syscall, void* arg1, void* arg2, void* arg3) + extern "C" long cpp_syscall_handler(int syscall, uintptr_t arg1, uintptr_t arg2, uintptr_t arg3, uintptr_t arg4, uintptr_t arg5) { Thread::current().set_in_syscall(true); asm volatile("sti"); + (void)arg5; + long ret = 0; switch (syscall) { @@ -81,28 +75,25 @@ namespace Kernel sys_exit(); break; case SYS_READ: - ret = sys_read((int)(uintptr_t)arg1, arg2, (size_t)(uintptr_t)arg3); + ret = sys_read((int)arg1, (void*)arg2, (size_t)arg3, (size_t)arg4); break; case SYS_WRITE: - ret = sys_write((int)(uintptr_t)arg1, arg2, (size_t)(uintptr_t)arg3); + ret = sys_write((int)arg1, (const void*)arg2, (size_t)arg3, (size_t)arg4); break; case SYS_TERMID: sys_termid((char*)arg1); break; case SYS_CLOSE: - ret = sys_close((int)(uintptr_t)arg1); - break; - case SYS_SEEK: - ret = sys_seek((int)(uintptr_t)arg1, (long)arg2); + ret = sys_close((int)arg1); break; case SYS_OPEN: - ret = sys_open((const char*)arg1, (int)(uintptr_t)arg2); + ret = sys_open((const char*)arg1, (int)arg2); break; case SYS_ALLOC: ret = sys_alloc((size_t)arg1); break; case SYS_FREE: - sys_free(arg1); + sys_free((void*)arg1); break; default: Kernel::panic("Unknown syscall {}", syscall); diff --git a/kernel/kernel/Terminal/TTY.cpp b/kernel/kernel/Terminal/TTY.cpp index e99c67bb2a..85a4b7ba9f 100644 --- a/kernel/kernel/Terminal/TTY.cpp +++ b/kernel/kernel/Terminal/TTY.cpp @@ -66,7 +66,7 @@ namespace Kernel while (true) { Input::KeyEvent event; - ASSERT(MUST(Process::current().read(fd, &event, sizeof(event))) == sizeof(event)); + ASSERT(MUST(Process::current().read(fd, &event, 0, sizeof(event))) == sizeof(event)); TTY::current()->on_key(event); } }, nullptr diff --git a/libc/stdio.cpp b/libc/stdio.cpp index 81db60ae28..457714add3 100644 --- a/libc/stdio.cpp +++ b/libc/stdio.cpp @@ -76,7 +76,7 @@ int fflush(FILE* file) if (file->buffer_index == 0) return 0; - if (syscall(SYS_WRITE, file->fd, file->buffer, file->buffer_index) < 0) + if (syscall(SYS_WRITE, file->fd, file->buffer, file->offset, file->buffer_index) < 0) { file->error = true; return EOF; @@ -92,7 +92,7 @@ int fgetc(FILE* file) return EOF; unsigned char c; - long ret = syscall(SYS_READ, file->fd, &c, 1); + long ret = syscall(SYS_READ, file->fd, &c, file->offset, 1); if (ret < 0) { @@ -235,7 +235,7 @@ size_t fread(void* buffer, size_t size, size_t nitems, FILE* file) { if (file->eof || nitems * size == 0) return 0; - long ret = syscall(SYS_READ, file->fd, buffer, size * nitems); + long ret = syscall(SYS_READ, file->fd, buffer, file->offset, size * nitems); if (ret < 0) { file->error = true; @@ -260,7 +260,13 @@ int fseek(FILE* file, long offset, int whence) int fseeko(FILE* file, off_t offset, int whence) { - if (whence == SEEK_CUR) + if (offset < 0) + { + errno = EINVAL; + return -1; + } + + if (whence == SEEK_CUR && offset <= file->offset) file->offset += offset; else if (whence == SEEK_SET) file->offset = offset; @@ -275,16 +281,6 @@ int fseeko(FILE* file, off_t offset, int whence) return -1; } - if (file->offset < 0) - { - file->offset -= offset; - errno = EINVAL; - return -1; - } - - if (syscall(SYS_SEEK, file->fd, file->offset)) - return -1; - file->eof = false; return 0; diff --git a/libc/unistd.cpp b/libc/unistd.cpp index 2ebb167dc1..19ecdc2a2d 100644 --- a/libc/unistd.cpp +++ b/libc/unistd.cpp @@ -31,22 +31,24 @@ long syscall(long syscall, ...) { int fd = va_arg(args, int); void* buffer = va_arg(args, void*); + size_t offset = va_arg(args, size_t); size_t bytes = va_arg(args, size_t); - ret = Kernel::syscall(SYS_READ, fd, buffer, bytes); + ret = Kernel::syscall(SYS_READ, fd, (uintptr_t)buffer, offset, bytes); break; } case SYS_WRITE: { int fd = va_arg(args, int); const char* string = va_arg(args, const char*); + size_t offset = va_arg(args, size_t); size_t bytes = va_arg(args, size_t); - ret = Kernel::syscall(SYS_WRITE, fd, string, bytes); + ret = Kernel::syscall(SYS_WRITE, fd, (uintptr_t)string, offset, bytes); break; } case SYS_TERMID: { char* buffer = va_arg(args, char*); - ret = Kernel::syscall(SYS_TERMID, buffer); + ret = Kernel::syscall(SYS_TERMID, (uintptr_t)buffer); break; } case SYS_CLOSE: @@ -55,18 +57,11 @@ long syscall(long syscall, ...) ret = Kernel::syscall(SYS_CLOSE, fd); break; } - case SYS_SEEK: - { - int fd = va_arg(args, int); - long offset = va_arg(args, long); - ret = Kernel::syscall(SYS_SEEK, fd, offset); - break; - } case SYS_OPEN: { const char* path = va_arg(args, const char*); int oflags = va_arg(args, int); - ret = Kernel::syscall(SYS_OPEN, path, oflags); + ret = Kernel::syscall(SYS_OPEN, (uintptr_t)path, oflags); break; } case SYS_ALLOC: @@ -78,7 +73,7 @@ long syscall(long syscall, ...) case SYS_FREE: { void* ptr = va_arg(args, void*); - ret = Kernel::syscall(SYS_FREE, ptr); + ret = Kernel::syscall(SYS_FREE, (uintptr_t)ptr); break; } default: diff --git a/userspace/test.c b/userspace/test.c index 3a6d3146f4..fe6306b508 100644 --- a/userspace/test.c +++ b/userspace/test.c @@ -1,35 +1,30 @@ #include #include -#define N 1024 +#define ERROR(msg) { perror(msg); return 1; } +#define BUF_SIZE 1024 int main() { - for (int i = 0; i <= 10; i++) - { - int** ptrs = malloc(N * sizeof(int*)); - if (ptrs == NULL) - { - perror("malloc"); - return 1; - } - for (int j = 0; j < N; j++) - { - ptrs[j] = malloc(sizeof(int)); - if (ptrs[j] == NULL) - { - perror("malloc"); - return 1; - } - *ptrs[j] = j; - putchar('0' + *ptrs[j] % 10); - } - putchar('\n'); + FILE* fp = fopen("/usr/include/stdio.h", "r"); + if (fp == NULL) + ERROR("fopen"); - for (int j = 0; j < N; j++) - free(ptrs[j]); - free(ptrs); + char* buffer = malloc(BUF_SIZE); + if (buffer == NULL) + ERROR("malloc"); + + for (;;) + { + size_t n_read = fread(buffer, 1, BUF_SIZE - 1, fp); + if (n_read == 0) + break; + buffer[n_read] = '\0'; + printf("%s", buffer); } - + + free(buffer); + fclose(fp); + return 0; }