Merge pull request #11986 from ARMmbed/feature_storage_test_improvements

Storage related test improvements and small fixes

UNITTESTS/moduletests/storage/blockdevice/BufferedBlockDevice/moduletest.cpp

@@ -0,0 +1,150 @@
+/* Copyright (c) 2019 ARM Limited
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "gtest/gtest.h"
+#include "features/storage/blockdevice/HeapBlockDevice.h"
+#include "features/storage/blockdevice/BufferedBlockDevice.h"
+
+#define BLOCK_SIZE (512)
+#define DEVICE_SIZE (BLOCK_SIZE*10)
+
+class BufferedBlockModuleTest : public testing::Test {
+protected:
+    HeapBlockDevice bd_heap{DEVICE_SIZE};
+    BufferedBlockDevice bd{&bd_heap};
+    uint8_t *magic;
+    uint8_t *buf;
+    virtual void SetUp()
+    {
+        ASSERT_EQ(bd.init(), 0);
+        magic = new uint8_t[BLOCK_SIZE];
+        buf = new uint8_t[BLOCK_SIZE];
+        // Generate simple pattern to verify against
+        for (int i = 0; i < BLOCK_SIZE; i++) {
+            magic[i] = 0xaa + i;
+        }
+    }
+
+    virtual void TearDown()
+    {
+        ASSERT_EQ(bd.deinit(), 0);
+        delete[] magic;
+        delete[] buf;
+    }
+};
+
+TEST_F(BufferedBlockModuleTest, init)
+{
+    BufferedBlockDevice b(&bd_heap);
+    EXPECT_EQ(b.get_erase_size(), 0);
+    EXPECT_EQ(b.get_erase_size(0), 0);
+    EXPECT_EQ(b.get_erase_value(), BD_ERROR_DEVICE_ERROR);
+    EXPECT_EQ(b.size(), 0);
+
+    EXPECT_EQ(b.init(), 0);
+
+    EXPECT_EQ(b.get_erase_size(), bd_heap.get_erase_size());
+    EXPECT_EQ(b.get_erase_size(0), bd_heap.get_erase_size(0));
+    EXPECT_EQ(b.get_erase_value(), bd_heap.get_erase_value());
+    EXPECT_EQ(b.get_program_size(), 1);
+    EXPECT_EQ(b.get_read_size(), 1);
+    EXPECT_EQ(b.size(), bd_heap.size());
+    EXPECT_EQ(b.get_type(), bd_heap.get_type());
+}
+
+TEST_F(BufferedBlockModuleTest, read_full_block)
+{
+    bd_heap.program(magic, 0, BLOCK_SIZE);
+    EXPECT_EQ(0, bd.read(buf, 0, BLOCK_SIZE));
+    EXPECT_EQ(0, memcmp(magic, buf, BLOCK_SIZE));
+}
+
+TEST_F(BufferedBlockModuleTest, over_read)
+{
+    bd_heap.program(magic, DEVICE_SIZE - BLOCK_SIZE, BLOCK_SIZE);
+    EXPECT_EQ(bd.read(buf, DEVICE_SIZE - BLOCK_SIZE, BLOCK_SIZE), 0);
+    EXPECT_EQ(bd.read(buf, DEVICE_SIZE, BLOCK_SIZE), BD_ERROR_DEVICE_ERROR);
+}
+
+TEST_F(BufferedBlockModuleTest, unalign_read)
+{
+    bd_heap.program(magic, 0, BLOCK_SIZE);
+    bd_heap.program(magic, BLOCK_SIZE, BLOCK_SIZE);
+    EXPECT_EQ(bd.read(buf, BLOCK_SIZE/2, BLOCK_SIZE), 0);
+    EXPECT_EQ(0, memcmp(buf, magic+(BLOCK_SIZE/2), BLOCK_SIZE/2));
+    EXPECT_EQ(0, memcmp(magic, buf+(BLOCK_SIZE/2), BLOCK_SIZE/2));
+}
+
+TEST_F(BufferedBlockModuleTest, align_big_read)
+{
+    uint8_t *buffer = new uint8_t[BLOCK_SIZE*2];
+    bd_heap.program(magic, 0, BLOCK_SIZE);
+    bd_heap.program(magic, BLOCK_SIZE, BLOCK_SIZE);
+    // Should cause 1 full block to be read unaligned from the device
+    // second block would require buffering, because it is not a full (-1)
+    EXPECT_EQ(bd.read(buffer, 0, (BLOCK_SIZE*2)-1), 0);
+    EXPECT_EQ(0, memcmp(magic, buffer, BLOCK_SIZE));
+    EXPECT_EQ(0, memcmp(magic, buffer+BLOCK_SIZE, BLOCK_SIZE-1));
+    delete[] buffer;
+}
+
+TEST_F(BufferedBlockModuleTest, program_small_chunks)
+{
+    for (int i=0; i < BLOCK_SIZE - 1; ++i) {
+        EXPECT_EQ(bd.program(magic+i, i, 1), 0);
+    }
+    EXPECT_EQ(bd.read(buf, 0, BLOCK_SIZE), 0);
+    EXPECT_EQ(0, memcmp(buf, magic, BLOCK_SIZE - 1));
+}
+
+TEST_F(BufferedBlockModuleTest, program_and_read_from_cache)
+{
+    bd_heap.program(magic, 0, BLOCK_SIZE);
+    EXPECT_EQ(bd.program("a", 0, 1), 0);
+    EXPECT_EQ(bd.read(buf, 0, BLOCK_SIZE), 0);
+    EXPECT_EQ('a', buf[0]);
+    EXPECT_EQ(0, memcmp(buf+1, magic+1, BLOCK_SIZE-1));
+}
+
+TEST_F(BufferedBlockModuleTest, program_and_verify_from_storage)
+{
+    EXPECT_EQ(bd.program(magic, 0, BLOCK_SIZE/2), 0);
+    EXPECT_EQ(bd.program(magic+BLOCK_SIZE/2, BLOCK_SIZE/2, BLOCK_SIZE/2), 0); // This should actually write to device
+    EXPECT_EQ(bd.read(buf, 0, BLOCK_SIZE), 0);
+    EXPECT_EQ(0, memcmp(buf, magic, BLOCK_SIZE));
+    // Verify that data actually is in the underlying block device
+    bd_heap.read(buf, 0, BLOCK_SIZE);
+    EXPECT_EQ(0, memcmp(buf, magic, BLOCK_SIZE));
+}
+
+TEST_F(BufferedBlockModuleTest, flush_automatically)
+{
+    EXPECT_EQ(bd.program(magic, 0, BLOCK_SIZE/2), 0); // Don't write full block
+    EXPECT_EQ(bd.program(magic, BLOCK_SIZE, BLOCK_SIZE/2), 0); // This should cause flush() for previous data in cache
+    // Verify that data actually is in the underlying block device
+    bd_heap.read(buf, 0, BLOCK_SIZE);
+    EXPECT_EQ(0, memcmp(buf, magic, BLOCK_SIZE/2));
+}
+
+TEST_F(BufferedBlockModuleTest, flush_at_exit)
+{
+    bd_heap.init(); // Extra init, to prevent deinit() when BufferedBlockDevice de-inits.
+    EXPECT_EQ(bd.program(magic, 0, BLOCK_SIZE/2), 0); // Don't write full block
+    EXPECT_EQ(bd.deinit(), 0);
+    // Verify that data actually is in the underlying block device
+    EXPECT_EQ(bd_heap.read(buf, 0, BLOCK_SIZE), 0);
+    EXPECT_EQ(0, memcmp(buf, magic, BLOCK_SIZE/2));
+}

UNITTESTS/moduletests/storage/blockdevice/BufferedBlockDevice/unittest.cmake

@@ -0,0 +1,20 @@
+
+####################
+# UNIT TESTS
+####################
+
+set(unittest-includes ${unittest-includes}
+  .
+  ..
+)
+
+set(unittest-sources
+  ../features/storage/blockdevice/BufferedBlockDevice.cpp
+  ../features/storage/blockdevice/HeapBlockDevice.cpp
+  stubs/mbed_atomic_stub.c
+  stubs/mbed_assert_stub.cpp
+)
+
+set(unittest-test-sources
+  moduletests/storage/blockdevice/BufferedBlockDevice/moduletest.cpp
+)

UNITTESTS/moduletests/storage/blockdevice/SlicingBlockDevice/moduletest.cpp

@@ -148,6 +148,7 @@ TEST_F(SlicingBlockModuleTest, slice_at_the_end)
 TEST_F(SlicingBlockModuleTest, over_write)
 {
     uint8_t *program = new uint8_t[BLOCK_SIZE] {0xbb,0xbb,0xbb};
+    uint8_t *buf = new uint8_t[BLOCK_SIZE];
 
     //Screate sclicing device, with size of 2 blocks
     mbed::SlicingBlockDevice slice(&bd, BLOCK_SIZE, BLOCK_SIZE * 3);
@@ -155,7 +156,59 @@ TEST_F(SlicingBlockModuleTest, over_write)
 
     EXPECT_EQ(slice.program(program, 0, BLOCK_SIZE), BD_ERROR_OK);
     EXPECT_EQ(slice.program(program, BLOCK_SIZE, BLOCK_SIZE), BD_ERROR_OK);
+
+    // Verify written value
+    EXPECT_EQ(slice.read(buf, BLOCK_SIZE, BLOCK_SIZE), BD_ERROR_OK);
+    EXPECT_EQ(0, memcmp(buf, program, BLOCK_SIZE));
+
     //Program a test value to address that is one pass the device size
     EXPECT_EQ(slice.program(program, 2 * BLOCK_SIZE, BLOCK_SIZE), BD_ERROR_DEVICE_ERROR);
     delete[] program;
 }
+
+TEST_F(SlicingBlockModuleTest, over_read)
+{
+    uint8_t *buf = new uint8_t[BLOCK_SIZE];
+
+    //Screate sclicing device, with size of 2 blocks
+    mbed::SlicingBlockDevice slice(&bd, BLOCK_SIZE, BLOCK_SIZE * 3);
+    EXPECT_EQ(slice.init(), BD_ERROR_OK);
+
+    //Try to read a block after the slice
+    EXPECT_EQ(slice.read(buf, 2 * BLOCK_SIZE, BLOCK_SIZE), BD_ERROR_DEVICE_ERROR);
+    delete[] buf;
+}
+
+TEST_F(SlicingBlockModuleTest, get_type)
+{
+    mbed::SlicingBlockDevice slice(&bd, BLOCK_SIZE, BLOCK_SIZE * 3);
+    EXPECT_EQ(bd.get_type(), slice.get_type());
+}
+
+TEST_F(SlicingBlockModuleTest, get_erase_value)
+{
+    mbed::SlicingBlockDevice slice(&bd, BLOCK_SIZE, BLOCK_SIZE * 3);
+    EXPECT_EQ(bd.get_erase_value(), slice.get_erase_value());
+}
+
+TEST_F(SlicingBlockModuleTest, erase)
+{
+    mbed::SlicingBlockDevice slice(&bd, BLOCK_SIZE, BLOCK_SIZE * 3);
+    EXPECT_EQ(slice.erase(0, BLOCK_SIZE), BD_ERROR_OK);
+    // Erase one block after the slice
+    EXPECT_EQ(slice.erase(2*BLOCK_SIZE, BLOCK_SIZE), BD_ERROR_DEVICE_ERROR);
+}
+
+TEST_F(SlicingBlockModuleTest, sync)
+{
+    mbed::SlicingBlockDevice slice(&bd, BLOCK_SIZE, BLOCK_SIZE * 3);
+    // Just a pass through
+    EXPECT_EQ(slice.sync(), 0);
+}
+
+TEST_F(SlicingBlockModuleTest, too_big_to_init)
+{
+    mbed::SlicingBlockDevice slice(&bd, 0, DEVICE_SIZE + BLOCK_SIZE);
+    // Just a pass through
+    EXPECT_EQ(slice.init(), BD_ERROR_DEVICE_ERROR);
+}

features/storage/TESTS/blockdevice/general_block_device/main.cpp

@@ -809,7 +809,7 @@ int main()
 
 
     Specification specification(greentea_test_setup, cases, total_num_cases,
-                                greentea_test_teardown_handler, (test_failure_handler_t)greentea_failure_handler);
+                                greentea_test_teardown_handler, default_handler);
 
     return !Harness::run(specification);
 }

features/storage/TESTS/kvstore/general_tests_phase_1/main.cpp

@@ -289,14 +289,21 @@ static void set_several_keys_multithreaded()
 //set key "write once" and try to set it again
 static void set_write_once_flag_try_set_twice()
 {
+    char buf[10];
+    size_t len;
     TEST_SKIP_UNLESS(kvstore != NULL);
 
-    int res = kvstore->set(key, data, data_size, KVStore::WRITE_ONCE_FLAG);
+    int res = kvstore->set(key, "ONCE", 5, KVStore::WRITE_ONCE_FLAG);
     TEST_ASSERT_EQUAL_ERROR_CODE(MBED_SUCCESS, res);
 
-    res = kvstore->set(key, data, data_size, KVStore::WRITE_ONCE_FLAG);
+    res = kvstore->set(key, "TWICE", 6, KVStore::WRITE_ONCE_FLAG);
     TEST_ASSERT_EQUAL_ERROR_CODE(MBED_ERROR_WRITE_PROTECTED, res);
 
+    res = kvstore->get(key, buf, 10, &len);
+    TEST_ASSERT_EQUAL_ERROR_CODE(MBED_SUCCESS, res);
+    TEST_ASSERT_EQUAL(len, 5);
+    TEST_ASSERT_EQUAL_STRING_LEN(buf, "ONCE", 5);
+
     res = kvstore->reset();
     TEST_ASSERT_EQUAL_ERROR_CODE(MBED_SUCCESS, res);
 }
@@ -304,14 +311,24 @@ static void set_write_once_flag_try_set_twice()
 //set key "write once" and try to remove it
 static void set_write_once_flag_try_remove()
 {
+    char buf[20];
+    size_t len;
     TEST_SKIP_UNLESS(kvstore != NULL);
 
-    int res = kvstore->set(key, data, data_size, KVStore::WRITE_ONCE_FLAG);
+    int res = kvstore->set(key, "TO_BE_REMOVED", 14, KVStore::WRITE_ONCE_FLAG);
     TEST_ASSERT_EQUAL_ERROR_CODE(MBED_SUCCESS, res);
 
+    res = kvstore->get(key, buf, 20, &len);
+    TEST_ASSERT_EQUAL_ERROR_CODE(MBED_SUCCESS, res);
+    TEST_ASSERT_EQUAL(len, 14);
+    TEST_ASSERT_EQUAL_STRING_LEN(buf, "TO_BE_REMOVED", 14);
+
     res = kvstore->remove(key);
     TEST_ASSERT_EQUAL_ERROR_CODE(MBED_ERROR_WRITE_PROTECTED, res);
 
+    res = kvstore->get(key, buf, 20, &len);
+    TEST_ASSERT_EQUAL_ERROR_CODE(MBED_SUCCESS, res);
+
     res = kvstore->reset();
     TEST_ASSERT_EQUAL_ERROR_CODE(MBED_SUCCESS, res);
 }
@@ -873,7 +890,7 @@ int main()
     }
 
     Specification specification(greentea_test_setup, cases, total_num_cases,
-                                greentea_test_teardown_handler, (test_failure_handler_t)greentea_failure_handler);
+                                greentea_test_teardown_handler, default_handler);
 
     return !Harness::run(specification);
 }

features/storage/TESTS/kvstore/general_tests_phase_2/main.cpp

@@ -68,13 +68,18 @@ static int kv_setup = TDBStoreSet;
 
 static const int heap_alloc_threshold_size = 4096;
 
+static inline uint32_t align_up(uint32_t val, uint32_t size)
+{
+    return (((val - 1) / size) + 1) * size;
+}
+
 /*----------------initialization------------------*/
 
 //init the blockdevice
 static void kvstore_init()
 {
     int res;
-    size_t erase_size, ul_bd_size, rbp_bd_size;
+    size_t program_size, erase_size, ul_bd_size, rbp_bd_size;
     BlockDevice *sec_bd;
 
     res = bd->init();
@@ -109,10 +114,17 @@ static void kvstore_init()
             flash_bd = new FlashSimBlockDevice(bd);
             sec_bd = flash_bd;
         }
+        res = sec_bd->init();
+        TEST_ASSERT_EQUAL_ERROR_CODE(MBED_SUCCESS, res);
 
-        erase_size  = sec_bd->get_erase_size();
-        ul_bd_size  = erase_size * 4;
-        rbp_bd_size = erase_size * 2;
+        program_size  = sec_bd->get_program_size();
+        erase_size = sec_bd->get_erase_size();
+        // We must be able to hold at least 10 small keys (20 program sectors) and master record + internal data
+        ul_bd_size  = align_up(program_size * 40, erase_size);
+        rbp_bd_size = align_up(program_size * 40, erase_size);
+
+        res = sec_bd->deinit();
+        TEST_ASSERT_EQUAL_ERROR_CODE(MBED_SUCCESS, res);
 
         ul_bd = new SlicingBlockDevice(sec_bd, 0, ul_bd_size);
         rbp_bd = new SlicingBlockDevice(sec_bd, ul_bd_size, ul_bd_size + rbp_bd_size);
@@ -861,7 +873,7 @@ int main()
     }
 
     Specification specification(greentea_test_setup, cases, total_num_cases,
-                                greentea_test_teardown_handler, (test_failure_handler_t)greentea_failure_handler);
+                                greentea_test_teardown_handler, default_handler);
 
     return !Harness::run(specification);
 }

features/storage/TESTS/kvstore/tdbstore_whitebox/main.cpp

@@ -455,165 +455,6 @@ static void multi_set_test()
     delete tdbs;
 }
 
-static void error_inject_test()
-{
-
-#if !defined(TARGET_K64F) && !defined(TARGET_MCU_PSOC6)
-    TEST_SKIP_MESSAGE("Kvstore API tests run only on K64F devices and PSoC 6");
-#endif
-
-    char *key;
-    uint8_t *get_buf, *set_buf, *exists;
-    size_t key_size = 8;
-    size_t data_size = 16;
-    size_t num_keys = 'Z' - 'A' + 1;
-    size_t num_blocks = 4;
-    size_t block_size = 1024;
-    size_t actual_data_size;
-    int result;
-    uint8_t set_iters = 20;
-    uint8_t i, key_ind;
-
-    key = new char[key_size + 1];
-    get_buf = new uint8_t[data_size];
-    set_buf = new uint8_t[data_size];
-    exists = new uint8_t[num_keys];
-
-    uint8_t *dummy = new (std::nothrow) uint8_t[heap_alloc_threshold_size];
-    TEST_SKIP_UNLESS_MESSAGE(dummy, "Not enough heap to run test");
-
-    // Don't use a non volatile BD here (won't work in this test)
-    HeapBlockDevice bd(num_blocks * block_size, 1,  1, block_size);
-    FlashSimBlockDevice flash_bd(&bd);
-
-    // Erase flash first, as we search the erase value later
-    result = flash_bd.init();
-    TEST_ASSERT_EQUAL_ERROR_CODE(MBED_SUCCESS, result);
-    // We need to skip the test if we don't have enough memory for the heap block device.
-    // However, this device allocates the erase units on the fly, so "erase" it via the flash
-    // simulator. A failure here means we haven't got enough memory.
-    result = flash_bd.erase(0, flash_bd.size());
-    TEST_SKIP_UNLESS_MESSAGE(!result, "Not enough heap to run test");
-    result = flash_bd.deinit();
-    TEST_ASSERT_EQUAL_ERROR_CODE(MBED_SUCCESS, result);
-
-    delete[] dummy;
-
-    TDBStore *tdbs = new TDBStore(&flash_bd);
-
-    result = tdbs->init();
-    TEST_ASSERT_EQUAL_ERROR_CODE(MBED_SUCCESS, result);
-
-    memset(exists, 0, num_keys);
-
-    for (i = 0; i < set_iters; i++) {
-        for (key_ind = 0; key_ind < num_keys; key_ind++) {
-            memset(key, 'A' + key_ind, key_size);
-            key[key_size] = '\0';
-            memset(set_buf, key_ind * i, data_size);
-            if ((key_ind != (num_keys - 1)) && exists[key_ind] && !(rand() % 3)) {
-                result = tdbs->remove(key);
-                exists[key_ind] = 0;
-            } else {
-                result = tdbs->set(key, set_buf, data_size, 0);
-                exists[key_ind] = 1;
-            }
-            TEST_ASSERT_EQUAL_ERROR_CODE(MBED_SUCCESS, result);
-        }
-    }
-
-    for (uint8_t get_iter = 0; get_iter < 2; get_iter++) {
-        for (key_ind = 0; key_ind < num_keys; key_ind++) {
-            memset(key, 'A' + key_ind, key_size);
-            key[key_size] = '\0';
-            if (key_ind == (num_keys - 1)) {
-                // last key will hold the previous version at the second iteration (after being crippled)
-                memset(set_buf, key_ind * (set_iters - get_iter - 1), data_size);
-            } else {
-                memset(set_buf, key_ind * (set_iters - 1), data_size);
-            }
-            result = tdbs->get(key, get_buf, data_size, &actual_data_size);
-            if (exists[key_ind]) {
-                TEST_ASSERT_EQUAL_ERROR_CODE(MBED_SUCCESS, result);
-                TEST_ASSERT_EQUAL(data_size, actual_data_size);
-                TEST_ASSERT_EQUAL_STRING_LEN(set_buf, get_buf, data_size);
-            } else {
-                TEST_ASSERT_EQUAL_ERROR_CODE(MBED_ERROR_ITEM_NOT_FOUND, result);
-            }
-        }
-
-        if (get_iter) {
-            break;
-        }
-
-        // Cripple the last key in both areas
-        uint8_t erase_val = (uint8_t) flash_bd.get_erase_value();
-        uint8_t x;
-        bd_addr_t addr = bd.size();
-
-        for (int area = 1; area >= 0; area--) {
-            int j;
-            bool found = false;
-
-            // look for last non blank
-            while (addr && !found) {
-                addr -= data_size;
-                bd.read(get_buf, addr, data_size);
-                for (j = data_size - 1; j >= 0; j--) {
-                    if (get_buf[j] != erase_val) {
-                        addr += j;
-                        x = get_buf[j] + 1;
-                        found = true;
-                        break;
-                    }
-                }
-            }
-
-            if (!found) {
-                break;
-            }
-
-            // Cripple last non blank in area
-            bd.program(&x, addr, 1);
-            addr -= j;
-
-            if ((area == 0) || !addr) {
-                break;
-            }
-
-            // Skip at least one blank erase unit
-            uint32_t num_blanks = 0;
-            memset(set_buf, erase_val, data_size);
-            while (addr && (num_blanks < block_size)) {
-                bd.read(get_buf, addr, data_size);
-                if (memcmp(get_buf, set_buf, data_size)) {
-                    num_blanks = 0;
-                } else {
-                    num_blanks += data_size;
-                }
-                addr -= data_size;
-            }
-        }
-
-        result = tdbs->get(key, get_buf, data_size, &actual_data_size);
-        TEST_ASSERT_EQUAL_ERROR_CODE(MBED_ERROR_INVALID_DATA_DETECTED, result);
-
-        result = tdbs->deinit();
-        TEST_ASSERT_EQUAL_ERROR_CODE(MBED_SUCCESS, result);
-
-        result = tdbs->init();
-        TEST_ASSERT_EQUAL_ERROR_CODE(MBED_SUCCESS, result);
-    }
-
-    delete[] key;
-    delete[] get_buf;
-    delete[] set_buf;
-    delete[] exists;
-
-    delete tdbs;
-}
-
-
 utest::v1::status_t greentea_failure_handler(const Case *const source, const failure_t reason)
 {
     greentea_case_failure_abort_handler(source, reason);
@@ -623,7 +464,6 @@ utest::v1::status_t greentea_failure_handler(const Case *const source, const fai
 Case cases[] = {
     Case("TDBStore: White box test",     white_box_test,    greentea_failure_handler),
     Case("TDBStore: Multiple set test",  multi_set_test,    greentea_failure_handler),
-    Case("TDBStore: Error inject test",  error_inject_test, greentea_failure_handler),
 };
 
 utest::v1::status_t greentea_test_setup(const size_t number_of_cases)

features/storage/blockdevice/BufferedBlockDevice.cpp

@@ -31,6 +31,7 @@ BufferedBlockDevice::BufferedBlockDevice(BlockDevice *bd)
     : _bd(bd), _bd_program_size(0), _bd_read_size(0), _bd_size(0), _write_cache_addr(0), _write_cache_valid(false),
       _write_cache(0), _read_buf(0), _init_ref_count(0), _is_initialized(false)
 {
+    MBED_ASSERT(_bd);
 }
 
 BufferedBlockDevice::~BufferedBlockDevice()
@@ -75,6 +76,12 @@ int BufferedBlockDevice::deinit()
         return BD_ERROR_OK;
     }
 
+    // Flush out all data from buffers
+    int err = sync();
+    if (err) {
+        return err;
+    }
+
     uint32_t val = core_util_atomic_decr_u32(&_init_ref_count, 1);
 
     if (val) {
@@ -133,6 +140,11 @@ int BufferedBlockDevice::read(void *b, bd_addr_t addr, bd_size_t size)
     }
 
     MBED_ASSERT(_write_cache && _read_buf);
+
+    if (!is_valid_read(addr, size)) {
+        return BD_ERROR_DEVICE_ERROR;
+    }
+
     // Common case - no need to involve write cache or read buffer
     if (_bd->is_valid_read(addr, size) &&
             ((addr + size <= _write_cache_addr) || (addr > _write_cache_addr + _bd_program_size))) {
@@ -145,9 +157,9 @@ int BufferedBlockDevice::read(void *b, bd_addr_t addr, bd_size_t size)
     while (size) {
         bd_size_t chunk;
         bool read_from_bd = true;
-        if (addr < _write_cache_addr) {
+        if (_write_cache_valid && addr < _write_cache_addr) {
             chunk = std::min(size, _write_cache_addr - addr);
-        } else if ((addr >= _write_cache_addr) && (addr < _write_cache_addr + _bd_program_size)) {
+        } else if (_write_cache_valid && (addr >= _write_cache_addr) && (addr < _write_cache_addr + _bd_program_size)) {
             // One case we need to take our data from cache
             chunk = std::min(size, _bd_program_size - addr % _bd_program_size);
             memcpy(buf, _write_cache + addr % _bd_program_size, chunk);
@@ -202,7 +214,6 @@ int BufferedBlockDevice::program(const void *b, bd_addr_t addr, bd_size_t size)
         if (ret) {
             return ret;
         }
-        _write_cache_addr = aligned_addr;
     }
 
     // Write logic: Keep data in cache as long as we don't reach the end of the program unit.
@@ -241,11 +252,11 @@ int BufferedBlockDevice::program(const void *b, bd_addr_t addr, bd_size_t size)
             if (ret) {
                 return ret;
             }
+            invalidate_write_cache();
             ret = _bd->sync();
             if (ret) {
                 return ret;
             }
-            invalidate_write_cache();
         } else {
             _write_cache_valid = true;
         }
@@ -332,11 +343,7 @@ bd_size_t BufferedBlockDevice::size() const
 
 const char *BufferedBlockDevice::get_type() const
 {
-    if (_bd != NULL) {
-        return _bd->get_type();
-    }
-
-    return NULL;
+    return _bd->get_type();
 }
 
 } // namespace mbed

features/storage/blockdevice/FlashSimBlockDevice.cpp

@@ -20,6 +20,7 @@
 #include <algorithm>
 #include <stdlib.h>
 #include <string.h>
+#include "mbed_assert.h"
 
 namespace mbed {
 
@@ -34,6 +35,7 @@ FlashSimBlockDevice::FlashSimBlockDevice(BlockDevice *bd, uint8_t erase_value) :
     _erase_value(erase_value), _blank_buf_size(0),
     _blank_buf(0), _bd(bd), _init_ref_count(0), _is_initialized(false)
 {
+    MBED_ASSERT(bd);
 }
 
 FlashSimBlockDevice::~FlashSimBlockDevice()

features/storage/blockdevice/HeapBlockDevice.cpp

@@ -87,31 +87,26 @@ int HeapBlockDevice::deinit()
 
 bd_size_t HeapBlockDevice::get_read_size() const
 {
-    MBED_ASSERT(_blocks != NULL);
     return _read_size;
 }
 
 bd_size_t HeapBlockDevice::get_program_size() const
 {
-    MBED_ASSERT(_blocks != NULL);
     return _program_size;
 }
 
 bd_size_t HeapBlockDevice::get_erase_size() const
 {
-    MBED_ASSERT(_blocks != NULL);
     return _erase_size;
 }
 
 bd_size_t HeapBlockDevice::get_erase_size(bd_addr_t addr) const
 {
-    MBED_ASSERT(_blocks != NULL);
     return _erase_size;
 }
 
 bd_size_t HeapBlockDevice::size() const
 {
-    MBED_ASSERT(_blocks != NULL);
     return _count * _erase_size;
 }
 
@@ -193,4 +188,3 @@ const char *HeapBlockDevice::get_type() const
 }
 
 } // namespace mbed
-

features/storage/blockdevice/SlicingBlockDevice.cpp

@@ -27,6 +27,11 @@ SlicingBlockDevice::SlicingBlockDevice(BlockDevice *bd, bd_addr_t start, bd_addr
     , _start_from_end(false), _start(start)
     , _stop_from_end(false), _stop(stop)
 {
+    MBED_ASSERT(bd);
+    // SlicingBlockDevice(bd, 0,0) would use the full block, which does not make sense, it must be a programming eror.
+    // SlicingBlockDevice(bd, 100,100) would have no size, which is also a programming error.
+    MBED_ASSERT(start != stop);
+
     if ((int64_t)_start < 0) {
         _start_from_end = true;
         _start = -_start;
@@ -147,12 +152,7 @@ bd_size_t SlicingBlockDevice::size() const
 
 const char *SlicingBlockDevice::get_type() const
 {
-    if (_bd != NULL) {
-        return _bd->get_type();
-    }
-
-    return NULL;
+    return _bd->get_type();
 }
 
 } // namespace mbed
-