From 90a7268e5a41ef8d1919dabf4c2664916375e9e5 Mon Sep 17 00:00:00 2001 From: Bananymous Date: Thu, 2 Mar 2023 12:30:11 +0200 Subject: [PATCH] BAN: Rewrite RefCounted to return ErrorOr --- BAN/include/BAN/Memory.h | 90 +++++++++++++++++++++------------------ kernel/kernel/FS/Ext2.cpp | 20 ++++++--- 2 files changed, 63 insertions(+), 47 deletions(-) diff --git a/BAN/include/BAN/Memory.h b/BAN/include/BAN/Memory.h index 289c9bc66..05854024a 100644 --- a/BAN/include/BAN/Memory.h +++ b/BAN/include/BAN/Memory.h @@ -54,16 +54,7 @@ namespace BAN class RefCounted { public: - RefCounted() { } - RefCounted(T* pointer) - { - if (pointer) - { - m_pointer = pointer; - m_count = new int32_t(1); - ASSERT(m_count); - } - } + RefCounted() = default; RefCounted(const RefCounted& other) { *this = other; @@ -74,18 +65,33 @@ namespace BAN } ~RefCounted() { - reset(); + clear(); } - template - static RefCounted create(Args... args) + template + static ErrorOr> adopt(U* data) { - return RefCounted(new T(forward(args)...), new int32_t(1)); + uint32_t* count = new uint32_t(1); + if (!count) + return Error::from_string("RefCounted: Could not allocate memory"); + return RefCounted((T*)data, count); + } + + template + static ErrorOr> create(Args... args) + { + uint32_t* count = new uint32_t(1); + if (!count) + return Error::from_string("RefCounted: Could not allocate memory"); + T* data = new T(forward(args)...); + if (!data) + return Error::from_string("RefCounted: Could not allocate memory"); + return RefCounted(data, count); } RefCounted& operator=(const RefCounted& other) { - reset(); + clear(); if (other) { m_pointer = other.m_pointer; @@ -97,53 +103,53 @@ namespace BAN RefCounted& operator=(RefCounted&& other) { - reset(); - m_pointer = other.m_pointer; - m_count = other.m_count; - other.m_pointer = nullptr; - other.m_count = nullptr; - if (!(*this)) - reset(); + clear(); + if (other) + { + m_pointer = other.m_pointer; + m_count = other.m_count; + other.m_pointer = nullptr; + other.m_count = nullptr; + } return *this; } - T& operator*() { return *m_pointer;} - const T& operator*() const { return *m_pointer;} + T* ptr() { return m_pointer; } + const T* ptr() const { return m_pointer; } - T* operator->() { return m_pointer; } - const T* operator->() const { return m_pointer; } + T& operator*() { return *ptr();} + const T& operator*() const { return *ptr();} - void reset() + T* operator->() { return ptr(); } + const T* operator->() const { return ptr(); } + + void clear() { - ASSERT(!m_count == !m_pointer); - if (!m_count) + if (!*this) return; + (*m_count)--; if (*m_count == 0) { - delete m_count; delete m_pointer; + delete m_count; } - m_count = nullptr; + m_pointer = nullptr; + m_count = nullptr; } operator bool() const { - ASSERT(!m_count == !m_pointer); - return m_count && *m_count > 0; - } - - bool operator==(const RefCounted& other) const - { - if (m_pointer != other.m_pointer) + if (!m_count && !m_pointer) return false; - ASSERT(m_count == other.m_count); - return !m_count || *m_count > 0; + ASSERT(m_count && m_pointer); + ASSERT(*m_count > 0); + return true; } private: - RefCounted(T* pointer, int32_t* count) + RefCounted(T* pointer, uint32_t* count) : m_pointer(pointer) , m_count(count) { @@ -152,7 +158,7 @@ namespace BAN private: T* m_pointer = nullptr; - int32_t* m_count = nullptr; + uint32_t* m_count = nullptr; }; } diff --git a/kernel/kernel/FS/Ext2.cpp b/kernel/kernel/FS/Ext2.cpp index 4036c7397..10d40b042 100644 --- a/kernel/kernel/FS/Ext2.cpp +++ b/kernel/kernel/FS/Ext2.cpp @@ -270,8 +270,10 @@ namespace Kernel BAN::StringView entry_name = BAN::StringView(entry->name, entry->name_len); if (entry->inode && file_name == entry_name) { - Ext2::Inode asked_inode = TRY(m_fs->read_inode(entry->inode)); - result = BAN::RefCounted(new Ext2Inode(m_fs, BAN::move(asked_inode), entry_name)); + Ext2Inode* inode = new Ext2Inode(m_fs, TRY(m_fs->read_inode(entry->inode)), entry_name); + if (inode == nullptr) + return BAN::Error::from_string("Could not allocate Ext2Inode"); + result = TRY(BAN::RefCounted::adopt(inode)); return false; } entry_addr += entry->rec_len; @@ -304,8 +306,11 @@ namespace Kernel { BAN::StringView entry_name = BAN::StringView(entry->name, entry->name_len); Ext2::Inode current_inode = TRY(m_fs->read_inode(entry->inode)); - auto ref_counted_inode = BAN::RefCounted(new Ext2Inode(m_fs, BAN::move(current_inode), entry_name)); - TRY(inodes.push_back(BAN::move(ref_counted_inode))); + + Ext2Inode* inode = new Ext2Inode(m_fs, BAN::move(current_inode), entry_name); + if (inode == nullptr) + return BAN::Error::from_string("Could not allocate memory for Ext2Inode"); + TRY(inodes.push_back(TRY(BAN::RefCounted::adopt(inode)))); } entry_addr += entry->rec_len; } @@ -423,7 +428,12 @@ namespace Kernel BAN::ErrorOr Ext2FS::initialize_root_inode() { - m_root_inode = BAN::RefCounted(new Ext2Inode(this, TRY(read_inode(Ext2::Enum::ROOT_INO)), "")); + + Ext2Inode* root_inode = new Ext2Inode(this, TRY(read_inode(Ext2::Enum::ROOT_INO)), ""); + if (root_inode == nullptr) + return BAN::Error::from_string("Could not allocate Ext2Inode"); + m_root_inode = TRY(BAN::RefCounted::adopt(root_inode)); + #if EXT2_DEBUG_PRINT dprintln("root inode:"); dprintln(" created {}", ext2_root_inode().ctime);