From 94968dfee4d4d7c2e59414330ed886975695be2c Mon Sep 17 00:00:00 2001 From: Dominik Ermel Date: Fri, 19 Feb 2021 12:22:52 +0000 Subject: [PATCH] storage/stream/flash: Don't update last erased page offset on failure The stream_flash_erase_page would update stream_flash_ctx member last_erased_page_start_offset, to page offset it attempted to erase, even if such operation failed. The commit changes this behaviour so that in case of failure the last_erased_page_start_offset would still hold previously, successfully, erase page offset. Signed-off-by: Dominik Ermel --- subsys/storage/stream/stream_flash.c | 3 +- .../storage/stream/stream_flash/src/main.c | 29 +++++++++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/subsys/storage/stream/stream_flash.c b/subsys/storage/stream/stream_flash.c index 9b5d01e4fe6..ed99116f92c 100644 --- a/subsys/storage/stream/stream_flash.c +++ b/subsys/storage/stream/stream_flash.c @@ -33,7 +33,6 @@ int stream_flash_erase_page(struct stream_flash_ctx *ctx, off_t off) return 0; } - ctx->last_erased_page_start_offset = page.start_offset; LOG_DBG("Erasing page at offset 0x%08lx", (long)page.start_offset); flash_write_protection_set(ctx->fdev, false); @@ -42,6 +41,8 @@ int stream_flash_erase_page(struct stream_flash_ctx *ctx, off_t off) if (rc != 0) { LOG_ERR("Error %d while erasing page", rc); + } else { + ctx->last_erased_page_start_offset = page.start_offset; } return rc; diff --git a/tests/subsys/storage/stream/stream_flash/src/main.c b/tests/subsys/storage/stream/stream_flash/src/main.c index c91e5f329b4..99e2d6985f7 100644 --- a/tests/subsys/storage/stream/stream_flash/src/main.c +++ b/tests/subsys/storage/stream/stream_flash/src/main.c @@ -348,6 +348,12 @@ static void test_stream_flash_buffered_write_whole_page(void) VERIFY_WRITTEN(page_size, page_size); } +/* Erase that never completes successfully */ +static int bad_erase(const struct device *dev, off_t offset, size_t size) +{ + return -EINVAL; +} + static void test_stream_flash_erase_page(void) { int rc; @@ -362,6 +368,29 @@ static void test_stream_flash_erase_page(void) zassert_equal(rc, 0, "expected success"); VERIFY_ERASED(FLASH_BASE, page_size); + + /* + * Test failure in erase does not change context. + * The test is done by replacing erase function of device API with fake + * one that returns with an error, invoking the erase procedure + * and than comparing state of context prior to call to the one after. + */ + struct device fake_dev = *ctx.fdev; + struct flash_driver_api fake_api = *(struct flash_driver_api *)ctx.fdev->api; + struct stream_flash_ctx bad_ctx = ctx; + struct stream_flash_ctx cmp_ctx; + + fake_api.erase = bad_erase; + fake_dev.api = &fake_api; + bad_ctx.fdev = &fake_dev; + /* Triger erase attempt */ + bad_ctx.last_erased_page_start_offset = FLASH_BASE - 16; + cmp_ctx = bad_ctx; + + rc = stream_flash_erase_page(&bad_ctx, FLASH_BASE); + zassert_equal(memcmp(&bad_ctx, &cmp_ctx, sizeof(bad_ctx)), 0, + "Ctx should not get altered"); + zassert_equal(rc, -EINVAL, "Expected failure"); } #else static void test_stream_flash_erase_page(void)