From 7458f68c389eb8a70e825f425c4c6d8612b647d0 Mon Sep 17 00:00:00 2001 From: Bananymous Date: Wed, 8 Mar 2023 17:05:37 +0200 Subject: [PATCH] BAN: Error can now be constructed from c_string or format string If the resulting string would overflow, we just truncate it to fit the error message buffer (128) bytes --- BAN/include/BAN/Errors.h | 24 +++++-- kernel/include/kernel/Shell.h | 2 +- kernel/kernel/FS/Ext2.cpp | 6 +- kernel/kernel/FS/VirtualFileSystem.cpp | 4 +- kernel/kernel/Font.cpp | 18 +++--- kernel/kernel/Shell.cpp | 83 ++++++++++--------------- kernel/kernel/Storage/ATAController.cpp | 18 +++--- kernel/kernel/Storage/StorageDevice.cpp | 6 +- 8 files changed, 77 insertions(+), 84 deletions(-) diff --git a/BAN/include/BAN/Errors.h b/BAN/include/BAN/Errors.h index 2f5fa696..3f76dc78 100644 --- a/BAN/include/BAN/Errors.h +++ b/BAN/include/BAN/Errors.h @@ -21,20 +21,32 @@ namespace BAN class Error { public: - static Error from_string(const char* message) + static Error from_c_string(const char* message) { - static_assert(sizeof(message) < 128); Error result; - strncpy(result.m_message, message, sizeof(m_message)); - result.m_message[sizeof(result.m_message) - 1] = '\0'; + strncpy(result.m_message, message, sizeof(Error::m_message)); + result.m_message[sizeof(Error::m_message) - 1] = '\0'; result.m_error_code = 0xFF; return result; } + template + static Error from_format(const char* format, Args&&... args) + { + char buffer[sizeof(Error::m_message)] {}; + size_t index = 0; + auto putc = [&](char ch) + { + if (index < sizeof(buffer) - 1) + buffer[index++] = ch; + }; + Formatter::print(putc, format, forward(args)...); + return from_c_string(buffer); + } static Error from_errno(int error) { Error result; - strncpy(result.m_message, strerror(error), sizeof(m_message)); - result.m_message[sizeof(result.m_message) - 1] = '\0'; + strncpy(result.m_message, strerror(error), sizeof(Error::m_message)); + result.m_message[sizeof(Error::m_message) - 1] = '\0'; result.m_error_code = error; return result; } diff --git a/kernel/include/kernel/Shell.h b/kernel/include/kernel/Shell.h index fdd26c26..4788dbb1 100644 --- a/kernel/include/kernel/Shell.h +++ b/kernel/include/kernel/Shell.h @@ -19,7 +19,7 @@ namespace Kernel private: void rerender_buffer() const; BAN::Vector parse_arguments(BAN::StringView) const; - void process_command(const BAN::Vector&); + BAN::ErrorOr process_command(const BAN::Vector&); void key_event_callback(Input::KeyEvent); private: diff --git a/kernel/kernel/FS/Ext2.cpp b/kernel/kernel/FS/Ext2.cpp index 203ef308..09272f5c 100644 --- a/kernel/kernel/FS/Ext2.cpp +++ b/kernel/kernel/FS/Ext2.cpp @@ -222,7 +222,7 @@ namespace Kernel } } - return BAN::Error::from_string("Inode did not contain enough blocks"); + return BAN::Error::from_c_string("Inode did not contain enough blocks"); } BAN::ErrorOr> Ext2Inode::read_all() @@ -355,7 +355,7 @@ namespace Kernel } if (m_superblock.magic != 0xEF53) - return BAN::Error::from_string("Not a ext2 filesystem"); + return BAN::Error::from_c_string("Not a ext2 filesystem"); if (m_superblock.rev_level < 1) { @@ -392,7 +392,7 @@ namespace Kernel uint32_t number_of_block_groups = BAN::Math::div_round_up(m_superblock.inodes_count, m_superblock.inodes_per_group); uint32_t number_of_block_groups_check = BAN::Math::div_round_up(m_superblock.blocks_count, m_superblock.blocks_per_group); if (number_of_block_groups != number_of_block_groups_check) - return BAN::Error::from_string("Ambiguous number of blocks"); + return BAN::Error::from_c_string("Ambiguous number of blocks"); uint32_t block_group_descriptor_table_block = m_superblock.first_data_block + 1; uint32_t block_group_descriptor_table_sector_count = BAN::Math::div_round_up(32u * number_of_block_groups, sector_size); diff --git a/kernel/kernel/FS/VirtualFileSystem.cpp b/kernel/kernel/FS/VirtualFileSystem.cpp index b94c5e87..589dd7c6 100644 --- a/kernel/kernel/FS/VirtualFileSystem.cpp +++ b/kernel/kernel/FS/VirtualFileSystem.cpp @@ -109,13 +109,13 @@ namespace Kernel if (m_root_inode) return {}; - return BAN::Error::from_string("Could not locate root partition"); + return BAN::Error::from_c_string("Could not locate root partition"); } BAN::ErrorOr> VirtualFileSystem::from_absolute_path(BAN::StringView path) { if (path.front() != '/') - return BAN::Error::from_string("Path must be an absolute path"); + return BAN::Error::from_c_string("Path must be an absolute path"); auto inode = root_inode(); auto path_parts = TRY(path.split('/')); diff --git a/kernel/kernel/Font.cpp b/kernel/kernel/Font.cpp index 555d3469..36e49e87 100644 --- a/kernel/kernel/Font.cpp +++ b/kernel/kernel/Font.cpp @@ -26,14 +26,14 @@ namespace Kernel BAN::ErrorOr Font::load(BAN::StringView path) { if (!VirtualFileSystem::is_initialized()) - return BAN::Error::from_string("Virtual Filesystem is not initialized"); + return BAN::Error::from_c_string("Virtual Filesystem is not initialized"); auto inode = TRY(VirtualFileSystem::get().from_absolute_path(path)); auto file_data = TRY(inode->read_all()); if (file_data.size() < 4) - return BAN::Error::from_string("Font file is too small"); + return BAN::Error::from_c_string("Font file is too small"); if (file_data[0] == 0x36 && file_data[1] == 0x04) return TRY(parse_psf1(file_data)); @@ -41,14 +41,14 @@ namespace Kernel if (file_data[0] == 0x72 && file_data[1] == 0xB5 && file_data[2] == 0x4A && file_data[3] == 0x86) return TRY(parse_psf2(file_data)); - return BAN::Error::from_string("Unsupported font format"); + return BAN::Error::from_c_string("Unsupported font format"); } BAN::ErrorOr Font::parse_psf1(const BAN::Vector& font_data) { if (font_data.size() < 4) - return BAN::Error::from_string("Font file is too small"); + return BAN::Error::from_c_string("Font file is too small"); struct PSF1Header { @@ -63,7 +63,7 @@ namespace Kernel uint32_t glyph_data_size = glyph_size * glyph_count; if (font_data.size() < sizeof(PSF1Header) + glyph_data_size) - return BAN::Error::from_string("Font file is too small"); + return BAN::Error::from_c_string("Font file is too small"); BAN::Vector glyph_data; TRY(glyph_data.resize(glyph_data_size)); @@ -109,7 +109,7 @@ namespace Kernel } if (glyph_index != glyph_count) - return BAN::Error::from_string("Font did not contain unicode entry for all glyphs"); + return BAN::Error::from_c_string("Font did not contain unicode entry for all glyphs"); } else { @@ -146,7 +146,7 @@ namespace Kernel }; if (font_data.size() < sizeof(PSF2Header)) - return BAN::Error::from_string("Font file is too small"); + return BAN::Error::from_c_string("Font file is too small"); PSF2Header header; header.magic = BAN::Math::little_endian_to_host(font_data.data() + 0); @@ -161,7 +161,7 @@ namespace Kernel uint32_t glyph_data_size = header.glyph_count * header.glyph_size; if (font_data.size() < glyph_data_size + header.header_size) - return BAN::Error::from_string("Font file is too small"); + return BAN::Error::from_c_string("Font file is too small"); BAN::Vector glyph_data; TRY(glyph_data.resize(glyph_data_size)); @@ -207,7 +207,7 @@ namespace Kernel } } if (glyph_index != header.glyph_count) - return BAN::Error::from_string("Font did not contain unicode entry for all glyphs"); + return BAN::Error::from_c_string("Font did not contain unicode entry for all glyphs"); } else { diff --git a/kernel/kernel/Shell.cpp b/kernel/kernel/Shell.cpp index fabc0560..e7d78058 100644 --- a/kernel/kernel/Shell.cpp +++ b/kernel/kernel/Shell.cpp @@ -137,7 +137,7 @@ argument_done: return result; } - void Shell::process_command(const Vector& arguments) + BAN::ErrorOr Shell::process_command(const Vector& arguments) { if (arguments.empty()) { @@ -146,7 +146,7 @@ argument_done: else if (arguments.front() == "date") { if (arguments.size() != 1) - return TTY_PRINTLN("'date' does not support command line arguments"); + return BAN::Error::from_c_string("'date' does not support command line arguments"); auto time = RTC::get_current_time(); TTY_PRINTLN("{}", time); } @@ -163,7 +163,7 @@ argument_done: else if (arguments.front() == "clear") { if (arguments.size() != 1) - return TTY_PRINTLN("'clear' does not support command line arguments"); + return BAN::Error::from_c_string("'clear' does not support command line arguments"); m_tty->clear(); m_tty->set_cursor_position(0, 0); } @@ -172,7 +172,7 @@ argument_done: auto new_args = arguments; new_args.remove(0); auto start = PIT::ms_since_boot(); - process_command(new_args); + TRY(process_command(new_args)); auto duration = PIT::ms_since_boot() - start; TTY_PRINTLN("took {} ms", duration); } @@ -182,39 +182,37 @@ argument_done: s_thread_spinlock.lock(); - auto thread_or_error = Thread::create( + auto thread = TRY(Thread::create( [this, &arguments] { auto args = arguments; args.remove(0); s_thread_spinlock.unlock(); PIT::sleep(5000); - process_command(args); + if (auto res = process_command(args); res.is_error()) + TTY_PRINTLN("{}", res.error()); } - ); - if (thread_or_error.is_error()) - return TTY_PRINTLN("{}", thread_or_error.error()); - - MUST(Scheduler::get().add_thread(thread_or_error.release_value())); + )); + TRY(Scheduler::get().add_thread(thread)); while (s_thread_spinlock.is_locked()); } else if (arguments.front() == "memory") { if (arguments.size() != 1) - return TTY_PRINTLN("'memory' does not support command line arguments"); + return BAN::Error::from_c_string("'memory' does not support command line arguments"); kmalloc_dump_info(); } else if (arguments.front() == "sleep") { if (arguments.size() != 1) - return TTY_PRINTLN("'sleep' does not support command line arguments"); + return BAN::Error::from_c_string("'sleep' does not support command line arguments"); PIT::sleep(5000); } else if (arguments.front() == "cpuinfo") { if (arguments.size() != 1) - return TTY_PRINTLN("'cpuinfo' does not support command line arguments"); + return BAN::Error::from_c_string("'cpuinfo' does not support command line arguments"); uint32_t ecx, edx; auto vendor = CPUID::get_vendor(); @@ -235,11 +233,11 @@ argument_done: else if (arguments.front() == "random") { if (arguments.size() != 1) - return TTY_PRINTLN("'random' does not support command line arguments"); + return BAN::Error::from_c_string("'random' does not support command line arguments"); uint32_t ecx, edx; CPUID::get_features(ecx, edx); if (!(ecx & CPUID::Features::ECX_RDRND)) - return TTY_PRINTLN("cpu does not support RDRAND instruction"); + return BAN::Error::from_c_string("cpu does not support RDRAND instruction"); for (int i = 0; i < 10; i++) { @@ -251,7 +249,7 @@ argument_done: else if (arguments.front() == "reboot") { if (arguments.size() != 1) - return TTY_PRINTLN("'reboot' does not support command line arguments"); + return BAN::Error::from_c_string("'reboot' does not support command line arguments"); uint8_t good = 0x02; while (good & 0x02) good = IO::inb(0x64); @@ -261,31 +259,24 @@ argument_done: else if (arguments.front() == "lspci") { if (arguments.size() != 1) - return TTY_PRINTLN("'lspci' does not support command line arguments"); + return BAN::Error::from_c_string("'lspci' does not support command line arguments"); for (auto& device : PCI::get().devices()) TTY_PRINTLN("{2H}:{2H}.{2H} {2H}", device.bus(), device.dev(), device.func(), device.class_code()); } else if (arguments.front() == "ls") { if (!VirtualFileSystem::is_initialized()) - return TTY_PRINTLN("VFS not initialized :("); + return BAN::Error::from_c_string("VFS not initialized :("); if (arguments.size() > 2) - return TTY_PRINTLN("usage: 'ls [path]'"); + return BAN::Error::from_c_string("usage: 'ls [path]'"); BAN::StringView path = (arguments.size() == 2) ? arguments[1].sv() : "/"; if (path.front() != '/') - return TTY_PRINTLN("ls currently works only with absolute paths"); + return BAN::Error::from_c_string("ls currently works only with absolute paths"); - auto directory_or_error = VirtualFileSystem::get().from_absolute_path(path); - if (directory_or_error.is_error()) - return TTY_PRINTLN("{}", directory_or_error.error()); - auto directory = directory_or_error.release_value(); - - auto inodes_or_error = directory->directory_inodes(); - if (inodes_or_error.is_error()) - return TTY_PRINTLN("{}", inodes_or_error.error()); - auto& inodes = inodes_or_error.value(); + auto directory = TRY(VirtualFileSystem::get().from_absolute_path(path)); + auto inodes = TRY(directory->directory_inodes()); auto mode_string = [](Inode::Mode mode) { @@ -314,43 +305,32 @@ argument_done: else if (arguments.front() == "cat") { if (!VirtualFileSystem::is_initialized()) - return TTY_PRINTLN("VFS not initialized :("); + return BAN::Error::from_c_string("VFS not initialized :("); if (arguments.size() != 2) - return TTY_PRINTLN("usage: 'cat path'"); + return BAN::Error::from_c_string("usage: 'cat path'"); - auto file_or_error = VirtualFileSystem::get().from_absolute_path(arguments[1]); - if (file_or_error.is_error()) - return TTY_PRINTLN("{}", file_or_error.error()); - auto file = file_or_error.release_value(); - - auto data_or_error = file->read_all(); - if (data_or_error.is_error()) - return TTY_PRINTLN("{}", data_or_error.error()); - auto data = data_or_error.release_value(); - + auto file = TRY(VirtualFileSystem::get().from_absolute_path(arguments[1])); + auto data = TRY(file->read_all()); TTY_PRINTLN("{}", BAN::StringView((const char*)data.data(), data.size())); } else if (arguments.front() == "loadfont") { if (!VirtualFileSystem::is_initialized()) - return TTY_PRINTLN("VFS not initialized :("); + return BAN::Error::from_c_string("VFS not initialized :("); if (arguments.size() != 2) - return TTY_PRINTLN("usage: 'loadfont font_path'"); - - auto font_or_error = Font::load(arguments[1]); - if (font_or_error.is_error()) - return TTY_PRINTLN("{}", font_or_error.error()); - auto font = font_or_error.release_value(); + return BAN::Error::from_c_string("usage: 'loadfont font_path'"); + auto font = TRY(Font::load(arguments[1])); m_tty->set_font(font); } else { - TTY_PRINTLN("unrecognized command '{}'", arguments.front()); + return BAN::Error::from_format("unrecognized command '{}'", arguments.front()); } + return {}; } void Shell::rerender_buffer() const @@ -416,7 +396,8 @@ argument_done: auto arguments = parse_arguments(current_buffer.sv()); if (!arguments.empty()) { - process_command(arguments); + if (auto res = process_command(arguments); res.is_error()) + TTY_PRINTLN("{}", res.error()); MUST(m_old_buffer.push_back(current_buffer)); m_buffer = m_old_buffer; MUST(m_buffer.push_back(""_sv)); diff --git a/kernel/kernel/Storage/ATAController.cpp b/kernel/kernel/Storage/ATAController.cpp index 12366a63..3ffa3a95 100644 --- a/kernel/kernel/Storage/ATAController.cpp +++ b/kernel/kernel/Storage/ATAController.cpp @@ -187,7 +187,7 @@ namespace Kernel BAN::ErrorOr ATAController::read(ATADevice* device, uint64_t lba, uint8_t sector_count, uint8_t* buffer) { if (lba + sector_count > device->lba_count) - return BAN::Error::from_string("Attempted to read outside of the device boundaries"); + return BAN::Error::from_c_string("Attempted to read outside of the device boundaries"); LockGuard _(m_lock); @@ -274,21 +274,21 @@ namespace Kernel { uint8_t err = read(ATA_PORT_ERROR); if (err & ATA_ERROR_AMNF) - return BAN::Error::from_string("Address mark not found."); + return BAN::Error::from_c_string("Address mark not found."); if (err & ATA_ERROR_TKZNF) - return BAN::Error::from_string("Track zero not found."); + return BAN::Error::from_c_string("Track zero not found."); if (err & ATA_ERROR_ABRT) - return BAN::Error::from_string("Aborted command."); + return BAN::Error::from_c_string("Aborted command."); if (err & ATA_ERROR_MCR) - return BAN::Error::from_string("Media change request."); + return BAN::Error::from_c_string("Media change request."); if (err & ATA_ERROR_IDNF) - return BAN::Error::from_string("ID not found."); + return BAN::Error::from_c_string("ID not found."); if (err & ATA_ERROR_MC) - return BAN::Error::from_string("Media changed."); + return BAN::Error::from_c_string("Media changed."); if (err & ATA_ERROR_UNC) - return BAN::Error::from_string("Uncorrectable data error."); + return BAN::Error::from_c_string("Uncorrectable data error."); if (err & ATA_ERROR_BBK) - return BAN::Error::from_string("Bad Block detected."); + return BAN::Error::from_c_string("Bad Block detected."); ASSERT_NOT_REACHED(); } diff --git a/kernel/kernel/Storage/StorageDevice.cpp b/kernel/kernel/Storage/StorageDevice.cpp index 40f1dd5d..feb94746 100644 --- a/kernel/kernel/Storage/StorageDevice.cpp +++ b/kernel/kernel/Storage/StorageDevice.cpp @@ -193,7 +193,7 @@ namespace Kernel GPTHeader header = parse_gpt_header(lba1); if (!is_valid_gpt_header(header, sector_size())) - return BAN::Error::from_string("Invalid GPT header"); + return BAN::Error::from_c_string("Invalid GPT header"); uint32_t size = header.partition_entry_count * header.partition_entry_size; if (uint32_t remainder = size % sector_size()) @@ -203,7 +203,7 @@ namespace Kernel TRY(read_sectors(header.partition_entry_lba, size / sector_size(), entry_array.data())); if (!is_valid_gpt_crc32(header, lba1, entry_array)) - return BAN::Error::from_string("Invalid crc3 in the GPT header"); + return BAN::Error::from_c_string("Invalid crc3 in the GPT header"); for (uint32_t i = 0; i < header.partition_entry_count; i++) { @@ -241,7 +241,7 @@ namespace Kernel { const uint32_t sectors_in_partition = m_lba_end - m_lba_start; if (lba + sector_count > sectors_in_partition) - return BAN::Error::from_string("Attempted to read outside of the partition boundaries"); + return BAN::Error::from_c_string("Attempted to read outside of the partition boundaries"); TRY(m_device.read_sectors(m_lba_start + lba, sector_count, buffer)); return {}; }