From 5ed3f7c6bf70db2e14c3575c7fab99c370003d02 Mon Sep 17 00:00:00 2001 From: Liav A Date: Mon, 25 Apr 2022 19:54:06 +0300 Subject: [PATCH] Kernel/Storage: Migrate the partition code to use the ErrorOr container That code used the old AK::Result container, which leads to overly complicated initialization flow when trying to figure out the correct partition table type. Instead, when using the ErrorOr container the code is much simpler and more understandable. --- .../Storage/Partition/EBRPartitionTable.cpp | 10 +++--- Kernel/Storage/Partition/EBRPartitionTable.h | 5 +-- .../Storage/Partition/GUIDPartitionTable.cpp | 8 ++--- Kernel/Storage/Partition/GUIDPartitionTable.h | 5 +-- .../Storage/Partition/MBRPartitionTable.cpp | 12 +++---- Kernel/Storage/Partition/MBRPartitionTable.h | 5 +-- Kernel/Storage/Partition/PartitionTable.h | 9 +----- Kernel/Storage/StorageManagement.cpp | 31 +++++++------------ Kernel/Storage/StorageManagement.h | 4 +-- 9 files changed, 39 insertions(+), 50 deletions(-) diff --git a/Kernel/Storage/Partition/EBRPartitionTable.cpp b/Kernel/Storage/Partition/EBRPartitionTable.cpp index d3889093a9..67a402062a 100644 --- a/Kernel/Storage/Partition/EBRPartitionTable.cpp +++ b/Kernel/Storage/Partition/EBRPartitionTable.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2020, Liav A. + * Copyright (c) 2020-2022, Liav A. * * SPDX-License-Identifier: BSD-2-Clause */ @@ -9,13 +9,13 @@ namespace Kernel { -Result, PartitionTable::Error> EBRPartitionTable::try_to_initialize(StorageDevice const& device) +ErrorOr> EBRPartitionTable::try_to_initialize(StorageDevice const& device) { - auto table = adopt_nonnull_own_or_enomem(new (nothrow) EBRPartitionTable(device)).release_value_but_fixme_should_propagate_errors(); + auto table = TRY(adopt_nonnull_own_or_enomem(new (nothrow) EBRPartitionTable(device))); if (table->is_protective_mbr()) - return { PartitionTable::Error::MBRProtective }; + return Error::from_errno(ENOTSUP); if (!table->is_valid()) - return { PartitionTable::Error::Invalid }; + return Error::from_errno(EINVAL); return table; } diff --git a/Kernel/Storage/Partition/EBRPartitionTable.h b/Kernel/Storage/Partition/EBRPartitionTable.h index bff3c70a53..6cce94358c 100644 --- a/Kernel/Storage/Partition/EBRPartitionTable.h +++ b/Kernel/Storage/Partition/EBRPartitionTable.h @@ -1,11 +1,12 @@ /* - * Copyright (c) 2020, Liav A. + * Copyright (c) 2020-2022, Liav A. * * SPDX-License-Identifier: BSD-2-Clause */ #pragma once +#include #include #include #include @@ -20,7 +21,7 @@ class EBRPartitionTable : public MBRPartitionTable { public: ~EBRPartitionTable(); - static Result, PartitionTable::Error> try_to_initialize(StorageDevice const&); + static ErrorOr> try_to_initialize(StorageDevice const&); explicit EBRPartitionTable(StorageDevice const&); virtual bool is_valid() const override { return m_valid; }; diff --git a/Kernel/Storage/Partition/GUIDPartitionTable.cpp b/Kernel/Storage/Partition/GUIDPartitionTable.cpp index 7605a20b2e..ff71737ea4 100644 --- a/Kernel/Storage/Partition/GUIDPartitionTable.cpp +++ b/Kernel/Storage/Partition/GUIDPartitionTable.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2020, Liav A. + * Copyright (c) 2020-2022, Liav A. * * SPDX-License-Identifier: BSD-2-Clause */ @@ -47,11 +47,11 @@ struct [[gnu::packed]] GUIDPartitionHeader { u32 crc32_entries_array; }; -Result, PartitionTable::Error> GUIDPartitionTable::try_to_initialize(StorageDevice const& device) +ErrorOr> GUIDPartitionTable::try_to_initialize(StorageDevice const& device) { - auto table = adopt_nonnull_own_or_enomem(new (nothrow) GUIDPartitionTable(device)).release_value_but_fixme_should_propagate_errors(); + auto table = TRY(adopt_nonnull_own_or_enomem(new (nothrow) GUIDPartitionTable(device))); if (!table->is_valid()) - return { PartitionTable::Error::Invalid }; + return Error::from_errno(EINVAL); return table; } diff --git a/Kernel/Storage/Partition/GUIDPartitionTable.h b/Kernel/Storage/Partition/GUIDPartitionTable.h index da90eb7454..1d2eeb0149 100644 --- a/Kernel/Storage/Partition/GUIDPartitionTable.h +++ b/Kernel/Storage/Partition/GUIDPartitionTable.h @@ -1,11 +1,12 @@ /* - * Copyright (c) 2020, Liav A. + * Copyright (c) 2020-2022, Liav A. * * SPDX-License-Identifier: BSD-2-Clause */ #pragma once +#include #include #include #include @@ -20,7 +21,7 @@ public: virtual ~GUIDPartitionTable() = default; ; - static Result, PartitionTable::Error> try_to_initialize(StorageDevice const&); + static ErrorOr> try_to_initialize(StorageDevice const&); explicit GUIDPartitionTable(StorageDevice const&); virtual bool is_valid() const override { return m_valid; }; diff --git a/Kernel/Storage/Partition/MBRPartitionTable.cpp b/Kernel/Storage/Partition/MBRPartitionTable.cpp index 6d81b5a5de..ad32867857 100644 --- a/Kernel/Storage/Partition/MBRPartitionTable.cpp +++ b/Kernel/Storage/Partition/MBRPartitionTable.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2020, Liav A. + * Copyright (c) 2020-2022, Liav A. * * SPDX-License-Identifier: BSD-2-Clause */ @@ -15,15 +15,15 @@ namespace Kernel { #define EBR_CHS_CONTAINER 0x05 #define EBR_LBA_CONTAINER 0x0F -Result, PartitionTable::Error> MBRPartitionTable::try_to_initialize(StorageDevice const& device) +ErrorOr> MBRPartitionTable::try_to_initialize(StorageDevice const& device) { - auto table = adopt_nonnull_own_or_enomem(new (nothrow) MBRPartitionTable(device)).release_value_but_fixme_should_propagate_errors(); + auto table = TRY(adopt_nonnull_own_or_enomem(new (nothrow) MBRPartitionTable(device))); if (table->contains_ebr()) - return { PartitionTable::Error::ContainsEBR }; + return Error::from_errno(ENOTSUP); if (table->is_protective_mbr()) - return { PartitionTable::Error::MBRProtective }; + return Error::from_errno(ENOTSUP); if (!table->is_valid()) - return { PartitionTable::Error::Invalid }; + return Error::from_errno(EINVAL); return table; } diff --git a/Kernel/Storage/Partition/MBRPartitionTable.h b/Kernel/Storage/Partition/MBRPartitionTable.h index dc10676b55..2fae2d6aba 100644 --- a/Kernel/Storage/Partition/MBRPartitionTable.h +++ b/Kernel/Storage/Partition/MBRPartitionTable.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2020, Liav A. + * Copyright (c) 2020-2022, Liav A. * * SPDX-License-Identifier: BSD-2-Clause */ @@ -7,6 +7,7 @@ #pragma once #include +#include #include #include #include @@ -41,7 +42,7 @@ public: public: ~MBRPartitionTable(); - static Result, PartitionTable::Error> try_to_initialize(StorageDevice const&); + static ErrorOr> try_to_initialize(StorageDevice const&); static OwnPtr try_to_initialize(StorageDevice const&, u32 start_lba); explicit MBRPartitionTable(StorageDevice const&); MBRPartitionTable(StorageDevice const&, u32 start_lba); diff --git a/Kernel/Storage/Partition/PartitionTable.h b/Kernel/Storage/Partition/PartitionTable.h index 34d955bd10..f9f4441f66 100644 --- a/Kernel/Storage/Partition/PartitionTable.h +++ b/Kernel/Storage/Partition/PartitionTable.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2020, Liav A. + * Copyright (c) 2020-2022, Liav A. * * SPDX-License-Identifier: BSD-2-Clause */ @@ -15,13 +15,6 @@ namespace Kernel { class PartitionTable { -public: - enum class Error { - Invalid, - MBRProtective, - ContainsEBR, - }; - public: Optional partition(unsigned index); size_t partitions_count() const { return m_partitions.size(); } diff --git a/Kernel/Storage/StorageManagement.cpp b/Kernel/Storage/StorageManagement.cpp index 4aa35875c1..2da6379b86 100644 --- a/Kernel/Storage/StorageManagement.cpp +++ b/Kernel/Storage/StorageManagement.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2020, Liav A. + * Copyright (c) 2020-2022, Liav A. * Copyright (c) 2022, the SerenityOS developers. * * SPDX-License-Identifier: BSD-2-Clause @@ -129,24 +129,16 @@ UNMAP_AFTER_INIT void StorageManagement::dump_storage_devices_and_partitions() c } } -UNMAP_AFTER_INIT OwnPtr StorageManagement::try_to_initialize_partition_table(StorageDevice const& device) const +UNMAP_AFTER_INIT ErrorOr> StorageManagement::try_to_initialize_partition_table(StorageDevice const& device) const { - auto mbr_table_or_result = MBRPartitionTable::try_to_initialize(device); - if (!mbr_table_or_result.is_error()) - return move(mbr_table_or_result.value()); - if (mbr_table_or_result.error() == PartitionTable::Error::MBRProtective) { - auto gpt_table_or_result = GUIDPartitionTable::try_to_initialize(device); - if (gpt_table_or_result.is_error()) - return {}; - return move(gpt_table_or_result.value()); + auto mbr_table_or_error = MBRPartitionTable::try_to_initialize(device); + if (!mbr_table_or_error.is_error()) + return mbr_table_or_error.release_value(); + auto ebr_table_or_error = EBRPartitionTable::try_to_initialize(device); + if (!ebr_table_or_error.is_error()) { + return ebr_table_or_error.release_value(); } - if (mbr_table_or_result.error() == PartitionTable::Error::ContainsEBR) { - auto ebr_table_or_result = EBRPartitionTable::try_to_initialize(device); - if (ebr_table_or_result.is_error()) - return {}; - return move(ebr_table_or_result.value()); - } - return {}; + return TRY(GUIDPartitionTable::try_to_initialize(device)); } UNMAP_AFTER_INIT void StorageManagement::enumerate_disk_partitions() @@ -154,9 +146,10 @@ UNMAP_AFTER_INIT void StorageManagement::enumerate_disk_partitions() VERIFY(!m_storage_devices.is_empty()); size_t device_index = 0; for (auto& device : m_storage_devices) { - auto partition_table = try_to_initialize_partition_table(device); - if (!partition_table) + auto partition_table_or_error = try_to_initialize_partition_table(device); + if (partition_table_or_error.is_error()) continue; + auto partition_table = partition_table_or_error.release_value(); for (size_t partition_index = 0; partition_index < partition_table->partitions_count(); partition_index++) { auto partition_metadata = partition_table->partition(partition_index); if (!partition_metadata.has_value()) diff --git a/Kernel/Storage/StorageManagement.h b/Kernel/Storage/StorageManagement.h index d37122fcc6..475a53540d 100644 --- a/Kernel/Storage/StorageManagement.h +++ b/Kernel/Storage/StorageManagement.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2020, Liav A. + * Copyright (c) 2020-2022, Liav A. * * SPDX-License-Identifier: BSD-2-Clause */ @@ -45,7 +45,7 @@ private: void dump_storage_devices_and_partitions() const; - OwnPtr try_to_initialize_partition_table(StorageDevice const&) const; + ErrorOr> try_to_initialize_partition_table(StorageDevice const&) const; RefPtr boot_block_device() const;