diff --git a/storage/blockdevice/include/blockdevice/HeapBlockDevice.h b/storage/blockdevice/include/blockdevice/HeapBlockDevice.h index 965896c427b..f853dc72cde 100644 --- a/storage/blockdevice/include/blockdevice/HeapBlockDevice.h +++ b/storage/blockdevice/include/blockdevice/HeapBlockDevice.h @@ -28,9 +28,11 @@ namespace mbed { -/** Lazily allocated heap-backed block device +/** Lazily allocated heap-backed block device. * - * Useful for simulating a block device and tests + * Useful for simulating a block device and tests. + * + * @note Each block is allocated when used, and freed when erased. * * @code * #include "mbed.h" diff --git a/storage/blockdevice/source/HeapBlockDevice.cpp b/storage/blockdevice/source/HeapBlockDevice.cpp index 65e5def5ded..996501b1413 100644 --- a/storage/blockdevice/source/HeapBlockDevice.cpp +++ b/storage/blockdevice/source/HeapBlockDevice.cpp @@ -40,11 +40,11 @@ HeapBlockDevice::~HeapBlockDevice() { if (_blocks) { for (size_t i = 0; i < _count; i++) { - free(_blocks[i]); + delete[] _blocks[i]; } delete[] _blocks; - _blocks = 0; + _blocks = nullptr; } } @@ -57,9 +57,13 @@ int HeapBlockDevice::init() } if (!_blocks) { - _blocks = new uint8_t *[_count]; + _blocks = new (std::nothrow) uint8_t *[_count]; + if (!_blocks) { + return BD_ERROR_DEVICE_ERROR; + } + for (size_t i = 0; i < _count; i++) { - _blocks[i] = 0; + _blocks[i] = nullptr; } } @@ -156,7 +160,7 @@ int HeapBlockDevice::program(const void *b, bd_addr_t addr, bd_size_t size) bd_addr_t lo = addr % _erase_size; if (!_blocks[hi]) { - _blocks[hi] = (uint8_t *)malloc(_erase_size); + _blocks[hi] = new (std::nothrow) uint8_t[_erase_size]; if (!_blocks[hi]) { return BD_ERROR_DEVICE_ERROR; } @@ -180,6 +184,13 @@ int HeapBlockDevice::erase(bd_addr_t addr, bd_size_t size) if (!is_valid_erase(addr, size)) { return BD_ERROR_DEVICE_ERROR; } + + for (size_t i = 0; i < (size / _erase_size); i++) { + size_t index = addr / _erase_size + i; + delete[] _blocks[index]; + _blocks[index] = nullptr; + } + return 0; } diff --git a/storage/kvstore/filesystemstore/tests/UNITTESTS/FileSystemStore/moduletest.cpp b/storage/kvstore/filesystemstore/tests/UNITTESTS/FileSystemStore/moduletest.cpp index 2896a6a846d..3c70b0911ed 100644 --- a/storage/kvstore/filesystemstore/tests/UNITTESTS/FileSystemStore/moduletest.cpp +++ b/storage/kvstore/filesystemstore/tests/UNITTESTS/FileSystemStore/moduletest.cpp @@ -67,21 +67,6 @@ TEST_F(FileSystemStoreModuleTest, set_get) EXPECT_STREQ("data", buf); } -TEST_F(FileSystemStoreModuleTest, erased_set_get) -{ - EXPECT_EQ(store->deinit(), MBED_SUCCESS); - EXPECT_EQ(heap.init(), MBED_SUCCESS); - EXPECT_EQ(heap.erase(0, heap.size()), MBED_SUCCESS); - EXPECT_EQ(heap.deinit(), MBED_SUCCESS); - EXPECT_EQ(store->init(), MBED_SUCCESS); - char buf[100]; - size_t size; - EXPECT_EQ(store->set("key", "data", 5, 0), MBED_SUCCESS); - EXPECT_EQ(store->get("key", buf, 100, &size), MBED_SUCCESS); - EXPECT_EQ(size, 5); - EXPECT_STREQ("data", buf); -} - TEST_F(FileSystemStoreModuleTest, set_deinit_init_get) { char buf[100]; diff --git a/storage/kvstore/tdbstore/include/tdbstore/TDBStore.h b/storage/kvstore/tdbstore/include/tdbstore/TDBStore.h index 75f68abbfac..bf7b4f0a0e2 100644 --- a/storage/kvstore/tdbstore/include/tdbstore/TDBStore.h +++ b/storage/kvstore/tdbstore/include/tdbstore/TDBStore.h @@ -346,14 +346,15 @@ class TDBStore : public KVStore { int reset_area(uint8_t area); /** - * @brief Erase an erase unit. + * @brief Erase an area. * * @param[in] area Area. * @param[in] offset Offset in area. + * @param[in] size Number of bytes to erase. * * @returns 0 for success, nonzero for failure. */ - int erase_erase_unit(uint8_t area, uint32_t offset); + int erase_area(uint8_t area, uint32_t offset, uint32_t size); /** * @brief Calculate addresses and sizes of areas. diff --git a/storage/kvstore/tdbstore/source/TDBStore.cpp b/storage/kvstore/tdbstore/source/TDBStore.cpp index 5d1539dcf5e..7d8d8822f32 100644 --- a/storage/kvstore/tdbstore/source/TDBStore.cpp +++ b/storage/kvstore/tdbstore/source/TDBStore.cpp @@ -170,24 +170,31 @@ int TDBStore::write_area(uint8_t area, uint32_t offset, uint32_t size, const voi return MBED_SUCCESS; } -int TDBStore::erase_erase_unit(uint8_t area, uint32_t offset) +int TDBStore::erase_area(uint8_t area, uint32_t offset, uint32_t size) { uint32_t bd_offset = _area_params[area].address + offset; - uint32_t eu_size = _buff_bd->get_erase_size(bd_offset); - if (_buff_bd->get_erase_value() != -1) { - return _buff_bd->erase(bd_offset, eu_size); - } else { - // We need to simulate erase, as our block device - // does not do it. We can do this one byte at a time - // because we use BufferedBlockDevice that has page buffers - uint8_t val = 0xff; - int ret; - for (; eu_size; --eu_size) { - ret = _buff_bd->program(&val, bd_offset++, 1); + int ret = _buff_bd->erase(bd_offset, size); + if (ret) { + return ret; + } + + if (_buff_bd->get_erase_value() == -1) { + // We need to simulate erase to wipe records, as our block device + // may not do it. Program in chunks of _work_buf_size if the minimum + // program size is too small (e.g. one-byte) to avoid performance + // issues. + MBED_ASSERT(_work_buf != nullptr); + MBED_ASSERT(_work_buf_size != 0); + memset(_work_buf, 0xFF, _work_buf_size); + while (size) { + uint32_t chunk = std::min(_work_buf_size, size); + ret = _buff_bd->program(_work_buf, bd_offset, chunk); if (ret) { return ret; } + size -= chunk; + bd_offset += chunk; } } return MBED_SUCCESS; @@ -1458,19 +1465,24 @@ void TDBStore::offset_in_erase_unit(uint8_t area, uint32_t offset, uint32_t &offset_from_start, uint32_t &dist_to_end) { uint32_t bd_offset = _area_params[area].address + offset; - uint32_t agg_offset = 0; - while (bd_offset >= agg_offset + _buff_bd->get_erase_size(agg_offset)) { - agg_offset += _buff_bd->get_erase_size(agg_offset); - } - offset_from_start = bd_offset - agg_offset; - dist_to_end = _buff_bd->get_erase_size(agg_offset) - offset_from_start; + // The parameter of `BlockDevice::get_erase_size(bd_addr_t addr)` + // does not need to be aligned. + uint32_t erase_unit = _buff_bd->get_erase_size(bd_offset); + + // Even on a flash device with multiple regions, the start address of + // an erase unit is aligned to the current region's unit size. + offset_from_start = bd_offset % erase_unit; + dist_to_end = erase_unit - offset_from_start; } int TDBStore::check_erase_before_write(uint8_t area, uint32_t offset, uint32_t size, bool force_check) { // In order to save init time, we don't check that the entire area is erased. // Instead, whenever reaching an erase unit start erase it. + bool erase = false; + uint32_t start_offset; + uint32_t end_offset; while (size) { uint32_t dist, offset_from_start; int ret; @@ -1478,13 +1490,22 @@ int TDBStore::check_erase_before_write(uint8_t area, uint32_t offset, uint32_t s uint32_t chunk = std::min(size, dist); if (offset_from_start == 0 || force_check) { - ret = erase_erase_unit(area, offset - offset_from_start); - if (ret != MBED_SUCCESS) { - return MBED_ERROR_WRITE_FAILED; + if (!erase) { + erase = true; + start_offset = offset - offset_from_start; } + end_offset = offset + dist; } offset += chunk; size -= chunk; } + + if (erase) { + int ret = erase_area(area, start_offset, end_offset - start_offset); + if (ret != MBED_SUCCESS) { + return MBED_ERROR_WRITE_FAILED; + } + } + return MBED_SUCCESS; } diff --git a/storage/kvstore/tdbstore/tests/TESTS/tdbstore/whitebox/main.cpp b/storage/kvstore/tdbstore/tests/TESTS/tdbstore/whitebox/main.cpp index da500b12dcb..11f820ca1ec 100644 --- a/storage/kvstore/tdbstore/tests/TESTS/tdbstore/whitebox/main.cpp +++ b/storage/kvstore/tdbstore/tests/TESTS/tdbstore/whitebox/main.cpp @@ -132,11 +132,14 @@ static void white_box_test() } else { test_bd = &heap_bd; // 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. - heap_bd.init(); - result = heap_bd.erase(0, heap_bd.size()); - TEST_SKIP_UNLESS_MESSAGE(!result, "Not enough heap to run test"); + // However, this device allocates the blocks on the fly when programmed. A failure + // here means we haven't got enough memory. + result = heap_bd.init(); + TEST_SKIP_UNLESS_MESSAGE(result == BD_ERROR_OK, "Not enough heap to run test") + for (bd_addr_t addr = 0; addr < heap_bd.size(); addr += heap_bd.get_program_size()) { + result = heap_bd.program(dummy, addr, heap_bd.get_program_size()); + TEST_SKIP_UNLESS_MESSAGE(result == BD_ERROR_OK, "Not enough heap to run test") + } heap_bd.deinit(); } @@ -345,11 +348,14 @@ static void multi_set_test() #ifdef USE_HEAP_BD // 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. - flash_bd.init(); - result = flash_bd.erase(0, flash_bd.size()); - TEST_SKIP_UNLESS_MESSAGE(!result, "Not enough heap to run test"); + // However, this device allocates the blocks on the fly when programmed. A failure + // here means we haven't got enough memory. + result = flash_bd.init(); + TEST_SKIP_UNLESS_MESSAGE(result == BD_ERROR_OK, "Not enough heap to run test") + for (bd_addr_t addr = 0; addr < flash_bd.size(); addr += flash_bd.get_program_size()) { + result = flash_bd.program(dummy, addr, flash_bd.get_program_size()); + TEST_SKIP_UNLESS_MESSAGE(result == BD_ERROR_OK, "Not enough heap to run test") + } flash_bd.deinit(); #endif