From d57f44fee072e22c9f101bdcf65fa0e01d97cb8b Mon Sep 17 00:00:00 2001 From: jimmorrisson Date: Thu, 10 Dec 2020 17:21:06 +0100 Subject: [PATCH] [EGD-4493] Unit test checks for disk manager. (#1118) --- module-vfs/src/purefs/blkdev/disk_manager.cpp | 72 +++++++-- module-vfs/tests/unittest_disk_manager.cpp | 148 ++++++++++++++++++ 2 files changed, 205 insertions(+), 15 deletions(-) diff --git a/module-vfs/src/purefs/blkdev/disk_manager.cpp b/module-vfs/src/purefs/blkdev/disk_manager.cpp index d269e6f81..c066a9432 100644 --- a/module-vfs/src/purefs/blkdev/disk_manager.cpp +++ b/module-vfs/src/purefs/blkdev/disk_manager.cpp @@ -21,22 +21,24 @@ namespace purefs::blkdev } // namespace auto disk_manager::register_device(std::shared_ptr disk, std::string_view device_name, unsigned flags) -> int { - { - cpp_freertos::LockGuard _lck(m_lock); - const auto ret = m_dev_map.find(std::string(device_name)); - if (ret != std::end(m_dev_map)) { - LOG_ERROR("Disc: %.*s already registered.", int(device_name.length()), device_name.data()); - return -EEXIST; - } - else { - auto ret = disk->probe(flags); - if (ret < 0) { - LOG_ERROR("Unable to probe the disc errno %i", ret); - return ret; - } - const auto it = m_dev_map.emplace(std::make_pair(device_name, disk)); - return reread_partitions(std::make_shared(disk, it.first->first)); + if (!disk) { + LOG_ERROR("Disk doesn't exists"); + return -EINVAL; + } + cpp_freertos::LockGuard _lck(m_lock); + const auto ret = m_dev_map.find(std::string(device_name)); + if (ret != std::end(m_dev_map)) { + LOG_ERROR("Disc: %.*s already registered.", int(device_name.length()), device_name.data()); + return -EEXIST; + } + else { + auto ret = disk->probe(flags); + if (ret < 0) { + LOG_ERROR("Unable to probe the disc errno %i", ret); + return ret; } + const auto it = m_dev_map.emplace(std::make_pair(device_name, disk)); + return reread_partitions(std::make_shared(disk, it.first->first)); } } auto disk_manager::unregister_device(std::string_view device_name) -> int @@ -131,6 +133,10 @@ namespace purefs::blkdev } auto disk_manager::write(disk_fd dfd, const void *buf, sector_t lba, std::size_t count) -> int { + if (!dfd) { + LOG_ERROR("Disk handle doesn't exists"); + return -EINVAL; + } auto disk = dfd->disk(); if (!disk) { LOG_ERROR("Disk doesn't exists"); @@ -146,6 +152,10 @@ namespace purefs::blkdev } auto disk_manager::read(disk_fd dfd, void *buf, sector_t lba, std::size_t count) -> int { + if (!dfd) { + LOG_ERROR("Disk handle doesn't exists"); + return -EINVAL; + } auto disk = dfd->disk(); if (!disk) { LOG_ERROR("Disk doesn't exists"); @@ -161,6 +171,10 @@ namespace purefs::blkdev } auto disk_manager::erase(disk_fd dfd, sector_t lba, std::size_t count) -> int { + if (!dfd) { + LOG_ERROR("Disk handle doesn't exists"); + return -EINVAL; + } auto disk = dfd->disk(); if (!disk) { LOG_ERROR("Disk doesn't exists"); @@ -176,6 +190,10 @@ namespace purefs::blkdev } auto disk_manager::sync(disk_fd dfd) -> int { + if (!dfd) { + LOG_ERROR("Disk handle doesn't exists"); + return -EINVAL; + } auto disk = dfd->disk(); if (!disk) { LOG_ERROR("Disk doesn't exists"); @@ -185,6 +203,10 @@ namespace purefs::blkdev } auto disk_manager::pm_control(disk_fd dfd, pm_state target_state) -> int { + if (!dfd) { + LOG_ERROR("Disk handle doesn't exists"); + return -EINVAL; + } auto disk = dfd->disk(); if (!disk) { LOG_ERROR("Disk doesn't exists"); @@ -194,6 +216,10 @@ namespace purefs::blkdev } auto disk_manager::pm_read(disk_fd dfd, pm_state ¤t_state) -> int { + if (!dfd) { + LOG_ERROR("Disk handle doesn't exists"); + return -EINVAL; + } auto disk = dfd->disk(); if (!disk) { LOG_ERROR("Disk doesn't exists"); @@ -203,6 +229,10 @@ namespace purefs::blkdev } auto disk_manager::status(disk_fd dfd) const -> media_status { + if (!dfd) { + LOG_ERROR("Disk handle doesn't exists"); + return media_status::error; + } auto disk = dfd->disk(); if (!disk) { LOG_ERROR("Disk doesn't exists"); @@ -212,6 +242,10 @@ namespace purefs::blkdev } auto disk_manager::partitions(disk_fd dfd) const -> std::vector { + if (!dfd) { + LOG_ERROR("Disk handle doesn't exists"); + return {}; + } auto disk = dfd->disk(); if (!disk) { LOG_ERROR("Disk doesn't exists"); @@ -221,6 +255,10 @@ namespace purefs::blkdev } auto disk_manager::get_info(disk_fd dfd, info_type what) const -> scount_t { + if (!dfd) { + LOG_ERROR("Disk handle doesn't exists"); + return {}; + } auto disk = dfd->disk(); if (!disk) { LOG_ERROR("Disk doesn't exists"); @@ -230,6 +268,10 @@ namespace purefs::blkdev } auto disk_manager::reread_partitions(disk_fd dfd) -> int { + if (!dfd) { + LOG_ERROR("Disk handle doesn't exists"); + return -EINVAL; + } auto disk = dfd->disk(); if (!disk) { LOG_ERROR("Disk doesn't exists"); diff --git a/module-vfs/tests/unittest_disk_manager.cpp b/module-vfs/tests/unittest_disk_manager.cpp index 641f96d23..95f5a2917 100644 --- a/module-vfs/tests/unittest_disk_manager.cpp +++ b/module-vfs/tests/unittest_disk_manager.cpp @@ -65,3 +65,151 @@ TEST_CASE("RW boundary checking") REQUIRE(dm.read(part1.name, buf2.data(), 0, 1) == 0); REQUIRE(buf1 == buf2); } + +TEST_CASE("Null pointer passed to disk manager functions") +{ + using namespace purefs; + blkdev::disk_manager dm; + + SECTION("Register device function") + { + std::shared_ptr disk = nullptr; + REQUIRE(dm.register_device(disk, "emmc0") == -EINVAL); + } + + SECTION("Write function") + { + REQUIRE(dm.write(static_cast(nullptr), + nullptr, + static_cast(0), + static_cast(0)) == -EINVAL); + } + + SECTION("Read function") + { + REQUIRE(dm.read(static_cast(nullptr), + nullptr, + static_cast(0), + static_cast(0)) == -EINVAL); + } + + SECTION("Erase function") + { + REQUIRE(dm.erase(static_cast(nullptr), 0, 0) == -EINVAL); + } + + SECTION("Sync function") + { + REQUIRE(dm.sync(static_cast(nullptr)) == -EINVAL); + } + + SECTION("PM control function") + { + REQUIRE(dm.pm_control(static_cast(nullptr), blkdev::pm_state::power_off) == -EINVAL); + } + + SECTION("Status function") + { + REQUIRE(dm.status(static_cast(nullptr)) == blkdev::media_status::error); + } + + SECTION("Partitions function") + { + REQUIRE(dm.partitions(static_cast(nullptr)).empty()); + } + + SECTION("Get info function") + { + REQUIRE(dm.get_info(static_cast(nullptr), blkdev::info_type::sector_size) == 0); + } + + SECTION("Reread partitions function") + { + REQUIRE(dm.reread_partitions(static_cast(nullptr)) == -EINVAL); + } +} + +TEST_CASE("Boundary checks for partitions") +{ + using namespace purefs; + blkdev::disk_manager dm; + + SECTION("Register device function") + { + REQUIRE(dm.unregister_device("") == -ENOENT); + } + + SECTION("Write function") + { + REQUIRE(dm.write("", nullptr, static_cast(0), static_cast(0)) == -ENOENT); + } + + SECTION("Read function") + { + REQUIRE(dm.read("", nullptr, static_cast(0), static_cast(0)) == -ENOENT); + } + + SECTION("Erase function") + { + REQUIRE(dm.erase("", 0, 0) == -ENOENT); + } + + SECTION("Sync function") + { + REQUIRE(dm.sync("") == -ENOENT); + } + + SECTION("PM control function") + { + REQUIRE(dm.pm_control("", blkdev::pm_state::power_off) == -ENOENT); + } + + SECTION("Status function") + { + REQUIRE(dm.status("") == blkdev::media_status::error); + } + + SECTION("Partitions function") + { + REQUIRE(dm.partitions("").empty()); + } + + SECTION("Get info function") + { + REQUIRE(dm.get_info("", blkdev::info_type::sector_size) == -ENOENT); + } + + SECTION("Reread partitions function") + { + REQUIRE(dm.reread_partitions("") == -ENOENT); + } +} + +TEST_CASE("Disk sectors out of range for partition") +{ + using namespace purefs; + blkdev::disk_manager dm; + auto disk = std::make_shared(disk_image); + REQUIRE(disk); + REQUIRE(dm.register_device(disk, "emmc1") == 0); + const auto parts = dm.partitions("emmc1"); + REQUIRE(parts.size() > 1); + const auto sect_size = dm.get_info("emmc1", blkdev::info_type::sector_size); + + SECTION("Read out of range") + { + std::vector buf(sect_size); + REQUIRE(dm.read("emmc1", buf.data(), parts[0].num_sectors - 1, parts[0].num_sectors) == -ERANGE); + } + + SECTION("Write out of range") + { + std::vector buf(sect_size); + REQUIRE(dm.write("emmc1", buf.data(), parts[0].num_sectors - 1, parts[0].num_sectors) == -ERANGE); + } + + SECTION("Erase out of range") + { + REQUIRE(dm.erase("emmc1", parts[0].num_sectors - 1, parts[0].num_sectors) == -ERANGE); + } +}