From cfb056901c345d6570dd9b76418a6297c4b913bc Mon Sep 17 00:00:00 2001 From: Dominik Ermel Date: Fri, 19 Feb 2021 12:28:14 +0000 Subject: [PATCH] storage/stream/flash: Fix flash_sync updating bytes_written on failure The flash_sync function is able to call, if specified, write verification callback to check whether data flash has been correctly written to a flash. Part of that procedure is to read date back of the flash and call the verification callback on the buffer; in case if the read would fail, the flash_sync would return, with an error code, without updating stream_flash_ctx. The same logic should be applied to failed verification, but, due to missing return, the stream_flash_ctx gets updated with probably incorrectly written bytes added to total bytes_written and buf_bytes, representing number of bytes awaiting in buffer, being zeroed. Signed-off-by: Dominik Ermel --- subsys/storage/stream/stream_flash.c | 1 + .../storage/stream/stream_flash/src/main.c | 34 +++++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/subsys/storage/stream/stream_flash.c b/subsys/storage/stream/stream_flash.c index ed99116f92c..0662b59df61 100644 --- a/subsys/storage/stream/stream_flash.c +++ b/subsys/storage/stream/stream_flash.c @@ -98,6 +98,7 @@ static int flash_sync(struct stream_flash_ctx *ctx) rc = ctx->callback(ctx->buf, ctx->buf_bytes, write_addr); if (rc != 0) { LOG_ERR("callback failed: %d", rc); + 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 99e2d6985f7..82e36c228c7 100644 --- a/tests/subsys/storage/stream/stream_flash/src/main.c +++ b/tests/subsys/storage/stream/stream_flash/src/main.c @@ -270,6 +270,16 @@ static void test_stream_flash_buf_size_greater_than_page_size(void) zassert_true(rc < 0, "expected failure"); } +static int bad_read(const struct device *dev, off_t off, void *data, size_t len) +{ + return -EINVAL; +} + +static int fake_write(const struct device *dev, off_t off, const void *data, size_t len) +{ + return 0; +} + static void test_stream_flash_buffered_write_callback(void) { int rc; @@ -304,6 +314,30 @@ static void test_stream_flash_buffered_write_callback(void) cb_buf = NULL; /* Don't verify other parameters of the callback */ rc = stream_flash_buffered_write(&ctx, write_buf, BUF_LEN, true); zassert_equal(rc, -EFAULT, "expected failure from callback"); + /* Expect that the BUF_LEN of bytes got stuck in buffer as the verification callback + * failed. + */ + zassert_equal(ctx.buf_bytes, BUF_LEN, "Expected bytes to be left in buffer"); + + 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.read = bad_read; + /* Using fake write here because after previous write, with faked callback failure, + * the flash is already written and real flash_write would cause failure. + */ + fake_api.write = fake_write; + fake_dev.api = &fake_api; + bad_ctx.fdev = &fake_dev; + /* Triger erase attempt */ + cmp_ctx = bad_ctx; + /* Just flush buffer */ + rc = stream_flash_buffered_write(&bad_ctx, write_buf, 0, true); + zassert_equal(rc, -EINVAL, "expected failure from flash_sync", rc); + zassert_equal(ctx.buf_bytes, BUF_LEN, "Expected bytes to be left in buffer"); + } static void test_stream_flash_flush(void)