From d9bc77e4226547dd82284e444de9be7f410a1840 Mon Sep 17 00:00:00 2001 From: "radek.tandler" Date: Wed, 19 Mar 2025 19:43:55 +0100 Subject: [PATCH] fix(nvs): fixed erasing of old values if new data type is not the same Closes https://github.com/espressif/esp-idf/issues/15559 --- .../host_test/nvs_host_test/main/test_nvs.cpp | 78 +++- components/nvs_flash/src/nvs_storage.cpp | 442 ++++++++++-------- 2 files changed, 330 insertions(+), 190 deletions(-) diff --git a/components/nvs_flash/host_test/nvs_host_test/main/test_nvs.cpp b/components/nvs_flash/host_test/nvs_host_test/main/test_nvs.cpp index 431e889203..b0762d9e54 100644 --- a/components/nvs_flash/host_test/nvs_host_test/main/test_nvs.cpp +++ b/components/nvs_flash/host_test/nvs_host_test/main/test_nvs.cpp @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2015-2024 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2015-2025 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -3806,6 +3806,82 @@ TEST_CASE("nvs multiple write with same key but different types", "[nvs]") TEST_ESP_OK(nvs_flash_deinit_partition(NVS_DEFAULT_PART_NAME)); } +TEST_CASE("nvs multiple write with same key blob and string involved", "[nvs]") +{ + PartitionEmulationFixture f(0, 10); + + nvs_handle_t handle_1; + const uint32_t NVS_FLASH_SECTOR = 6; + const uint32_t NVS_FLASH_SECTOR_COUNT_MIN = 3; + TEMPORARILY_DISABLED(f.emu.setBounds(NVS_FLASH_SECTOR, NVS_FLASH_SECTOR + NVS_FLASH_SECTOR_COUNT_MIN);) + + for (uint16_t j = NVS_FLASH_SECTOR; j < NVS_FLASH_SECTOR + NVS_FLASH_SECTOR_COUNT_MIN; ++j) { + f.erase(j); + } + TEST_ESP_OK(nvs::NVSPartitionManager::get_instance()->init_custom(f.part(), + NVS_FLASH_SECTOR, + NVS_FLASH_SECTOR_COUNT_MIN)); + + TEST_ESP_OK(nvs_open("namespace1", NVS_READWRITE, &handle_1)); + + nvs_erase_all(handle_1); + + const char key_name[] = "foo"; + + // integer variables + int32_t v32; + int8_t v8; + + // string + #define str_data_len 64 + const char str_data[] = "string data"; + char str_buf[str_data_len] = {0}; + size_t str_len = str_data_len; + + // blob + #define blob_data_len 64 + uint8_t blob_data[blob_data_len] = {0}; + uint8_t blob_buf[blob_data_len] = {0}; + size_t blob_read_size; + + // first write is i32 + TEST_ESP_OK(nvs_set_i32(handle_1, key_name, (int32_t)12345678)); + + TEST_ESP_ERR(nvs_get_i8(handle_1, key_name, &v8), ESP_ERR_NVS_NOT_FOUND); + TEST_ESP_OK(nvs_get_i32(handle_1, key_name, &v32)); + TEST_ESP_ERR(nvs_get_str(handle_1, key_name, str_buf, &str_len), ESP_ERR_NVS_NOT_FOUND); + TEST_ESP_ERR(nvs_get_blob(handle_1, key_name, blob_buf, &blob_read_size), ESP_ERR_NVS_NOT_FOUND); + + + // second write is string + TEST_ESP_OK(nvs_set_str(handle_1, key_name, str_data)); + + TEST_ESP_ERR(nvs_get_i8(handle_1, key_name, &v8), ESP_ERR_NVS_NOT_FOUND); + TEST_ESP_ERR(nvs_get_i32(handle_1, key_name, &v32), ESP_ERR_NVS_NOT_FOUND); + TEST_ESP_OK(nvs_get_str(handle_1, key_name, str_buf, &str_len)); + TEST_ESP_ERR(nvs_get_blob(handle_1, key_name, blob_buf, &blob_read_size), ESP_ERR_NVS_NOT_FOUND); + + // third write is blob + TEST_ESP_OK(nvs_set_blob(handle_1, key_name, blob_data, blob_data_len)); + + TEST_ESP_ERR(nvs_get_i8(handle_1, key_name, &v8), ESP_ERR_NVS_NOT_FOUND); + TEST_ESP_ERR(nvs_get_i32(handle_1, key_name, &v32), ESP_ERR_NVS_NOT_FOUND); + TEST_ESP_ERR(nvs_get_str(handle_1, key_name, str_buf, &str_len), ESP_ERR_NVS_NOT_FOUND); + TEST_ESP_OK(nvs_get_blob(handle_1, key_name, blob_buf, &blob_read_size)); + + // fourth write is i8 + TEST_ESP_OK(nvs_set_i8(handle_1, key_name, (int8_t)12)); + + TEST_ESP_OK(nvs_get_i8(handle_1, key_name, &v8)); + TEST_ESP_ERR(nvs_get_i32(handle_1, key_name, &v32), ESP_ERR_NVS_NOT_FOUND); + TEST_ESP_ERR(nvs_get_str(handle_1, key_name, str_buf, &str_len), ESP_ERR_NVS_NOT_FOUND); + TEST_ESP_ERR(nvs_get_blob(handle_1, key_name, blob_buf, &blob_read_size), ESP_ERR_NVS_NOT_FOUND); + + nvs_close(handle_1); + + TEST_ESP_OK(nvs_flash_deinit_partition(NVS_DEFAULT_PART_NAME)); +} + TEST_CASE("nvs find key tests", "[nvs]") { const size_t buff_len = 4096; diff --git a/components/nvs_flash/src/nvs_storage.cpp b/components/nvs_flash/src/nvs_storage.cpp index e5db0caefe..1cb35d8a38 100644 --- a/components/nvs_flash/src/nvs_storage.cpp +++ b/components/nvs_flash/src/nvs_storage.cpp @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2015-2024 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2015-2025 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -51,7 +51,7 @@ void Storage::clearNamespaces() esp_err_t Storage::populateBlobIndices(TBlobIndexList& blobIdxList) { - for (auto it = mPageManager.begin(); it != mPageManager.end(); ++it) { + for(auto it = mPageManager.begin(); it != mPageManager.end(); ++it) { Page& p = *it; size_t itemIndex = 0; Item item; @@ -60,10 +60,10 @@ esp_err_t Storage::populateBlobIndices(TBlobIndexList& blobIdxList) * logic in pagemanager will remove the earlier index. So we should never find a * duplicate index at this point */ - while (p.findItem(Page::NS_ANY, ItemType::BLOB_IDX, nullptr, itemIndex, item) == ESP_OK) { + while(p.findItem(Page::NS_ANY, ItemType::BLOB_IDX, nullptr, itemIndex, item) == ESP_OK) { BlobIndexNode* entry = new (std::nothrow) BlobIndexNode; - if (!entry) return ESP_ERR_NO_MEM; + if(!entry) return ESP_ERR_NO_MEM; item.getKey(entry->key, sizeof(entry->key)); entry->nsIndex = item.nsIndex; @@ -89,7 +89,7 @@ esp_err_t Storage::populateBlobIndices(TBlobIndexList& blobIdxList) // later by the call to eraseOrphanDataBlobs(). void Storage::eraseMismatchedBlobIndexes(TBlobIndexList& blobIdxList) { - for (auto it = mPageManager.begin(); it != mPageManager.end(); ++it) { + for(auto it = mPageManager.begin(); it != mPageManager.end(); ++it) { Page& p = *it; size_t itemIndex = 0; Item item; @@ -98,7 +98,7 @@ void Storage::eraseMismatchedBlobIndexes(TBlobIndexList& blobIdxList) * 1) VER_0_OFFSET <= chunkIndex < VER_1_OFFSET-1 => Version0 chunks * 2) VER_1_OFFSET <= chunkIndex < VER_ANY => Version1 chunks */ - while (p.findItem(Page::NS_ANY, ItemType::BLOB_DATA, nullptr, itemIndex, item) == ESP_OK) { + while(p.findItem(Page::NS_ANY, ItemType::BLOB_DATA, nullptr, itemIndex, item) == ESP_OK) { auto iter = std::find_if(blobIdxList.begin(), blobIdxList.end(), @@ -107,7 +107,7 @@ void Storage::eraseMismatchedBlobIndexes(TBlobIndexList& blobIdxList) && (item.nsIndex == e.nsIndex) && (item.chunkIndex >= static_cast (e.chunkStart)) && (item.chunkIndex < static_cast ((e.chunkStart == nvs::VerOffset::VER_0_OFFSET) ? nvs::VerOffset::VER_1_OFFSET : nvs::VerOffset::VER_ANY));}); - if (iter != std::end(blobIdxList)) { + if(iter != std::end(blobIdxList)) { // accumulate the size iter->observedDataSize += item.varLength.dataSize; iter->observedChunkCount++; @@ -117,15 +117,15 @@ void Storage::eraseMismatchedBlobIndexes(TBlobIndexList& blobIdxList) } auto iter = blobIdxList.begin(); - while (iter != blobIdxList.end()) + while(iter != blobIdxList.end()) { - if ( (iter->observedDataSize != iter->dataSize) || (iter->observedChunkCount != iter->chunkCount) ) + if( (iter->observedDataSize != iter->dataSize) || (iter->observedChunkCount != iter->chunkCount) ) { // Delete blob_index from flash // This is very rare case, so we can loop over all pages - for (auto it = mPageManager.begin(); it != mPageManager.end(); ++it) { + for(auto it = mPageManager.begin(); it != mPageManager.end(); ++it) { // skip pages in non eligible states - if (it->state() == nvs::Page::PageState::CORRUPT + if(it->state() == nvs::Page::PageState::CORRUPT || it->state() == nvs::Page::PageState::INVALID || it->state() == nvs::Page::PageState::UNINITIALIZED){ continue; @@ -153,7 +153,7 @@ void Storage::eraseMismatchedBlobIndexes(TBlobIndexList& blobIdxList) void Storage::eraseOrphanDataBlobs(TBlobIndexList& blobIdxList) { - for (auto it = mPageManager.begin(); it != mPageManager.end(); ++it) { + for(auto it = mPageManager.begin(); it != mPageManager.end(); ++it) { Page& p = *it; size_t itemIndex = 0; Item item; @@ -162,7 +162,7 @@ void Storage::eraseOrphanDataBlobs(TBlobIndexList& blobIdxList) * 1) VER_0_OFFSET <= chunkIndex < VER_1_OFFSET-1 => Version0 chunks * 2) VER_1_OFFSET <= chunkIndex < VER_ANY => Version1 chunks */ - while (p.findItem(Page::NS_ANY, ItemType::BLOB_DATA, nullptr, itemIndex, item) == ESP_OK) { + while(p.findItem(Page::NS_ANY, ItemType::BLOB_DATA, nullptr, itemIndex, item) == ESP_OK) { auto iter = std::find_if(blobIdxList.begin(), blobIdxList.end(), @@ -171,7 +171,7 @@ void Storage::eraseOrphanDataBlobs(TBlobIndexList& blobIdxList) && (item.nsIndex == e.nsIndex) && (item.chunkIndex >= static_cast (e.chunkStart)) && (item.chunkIndex < static_cast (e.chunkStart) + e.chunkCount);}); - if (iter == std::end(blobIdxList)) { + if(iter == std::end(blobIdxList)) { p.eraseItem(item.nsIndex, item.datatype, item.key, item.chunkIndex); } @@ -183,7 +183,7 @@ void Storage::eraseOrphanDataBlobs(TBlobIndexList& blobIdxList) esp_err_t Storage::init(uint32_t baseSector, uint32_t sectorCount) { auto err = mPageManager.load(mPartition, baseSector, sectorCount); - if (err != ESP_OK) { + if(err != ESP_OK) { mState = StorageState::INVALID; return err; } @@ -191,25 +191,25 @@ esp_err_t Storage::init(uint32_t baseSector, uint32_t sectorCount) // load namespaces list clearNamespaces(); std::fill_n(mNamespaceUsage.data(), mNamespaceUsage.byteSize() / 4, 0); - for (auto it = mPageManager.begin(); it != mPageManager.end(); ++it) { + for(auto it = mPageManager.begin(); it != mPageManager.end(); ++it) { Page& p = *it; size_t itemIndex = 0; Item item; - while (p.findItem(Page::NS_INDEX, ItemType::U8, nullptr, itemIndex, item) == ESP_OK) { + while(p.findItem(Page::NS_INDEX, ItemType::U8, nullptr, itemIndex, item) == ESP_OK) { NamespaceEntry* entry = new (std::nothrow) NamespaceEntry; - if (!entry) { + if(!entry) { mState = StorageState::INVALID; return ESP_ERR_NO_MEM; } item.getKey(entry->mName, sizeof(entry->mName)); err = item.getValue(entry->mIndex); - if (err != ESP_OK) { + if(err != ESP_OK) { delete entry; return err; } - if (mNamespaceUsage.set(entry->mIndex, true) != ESP_OK) { + if(mNamespaceUsage.set(entry->mIndex, true) != ESP_OK) { delete entry; return ESP_FAIL; } @@ -217,17 +217,17 @@ esp_err_t Storage::init(uint32_t baseSector, uint32_t sectorCount) itemIndex += item.span; } } - if (mNamespaceUsage.set(0, true) != ESP_OK) { + if(mNamespaceUsage.set(0, true) != ESP_OK) { return ESP_FAIL; } - if (mNamespaceUsage.set(255, true) != ESP_OK) { + if(mNamespaceUsage.set(255, true) != ESP_OK) { return ESP_FAIL; } // Populate list of multi-page index entries. TBlobIndexList blobIdxList; err = populateBlobIndices(blobIdxList); - if (err != ESP_OK) { + if(err != ESP_OK) { mState = StorageState::INVALID; return ESP_ERR_NO_MEM; } @@ -256,10 +256,10 @@ bool Storage::isValid() const esp_err_t Storage::findItem(uint8_t nsIndex, ItemType datatype, const char* key, Page* &page, Item& item, uint8_t chunkIdx, VerOffset chunkStart) { - for (auto it = std::begin(mPageManager); it != std::end(mPageManager); ++it) { + for(auto it = std::begin(mPageManager); it != std::end(mPageManager); ++it) { size_t itemIndex = 0; auto err = it->findItem(nsIndex, datatype, key, itemIndex, item, chunkIdx, chunkStart); - if (err == ESP_OK) { + if(err == ESP_OK) { page = it; return ESP_OK; } @@ -282,7 +282,7 @@ esp_err_t Storage::writeMultiPageBlob(uint8_t nsIndex, const char* key, const vo max_pages = (Page::CHUNK_ANY-1)/2; } - if (dataSize > max_pages * Page::CHUNK_MAX_SIZE) { + if(dataSize > max_pages * Page::CHUNK_MAX_SIZE) { return ESP_ERR_NVS_VALUE_TOO_LONG; } @@ -290,16 +290,16 @@ esp_err_t Storage::writeMultiPageBlob(uint8_t nsIndex, const char* key, const vo Page& page = getCurrentPage(); size_t tailroom = page.getVarDataTailroom(); size_t chunkSize = 0; - if (chunkCount == 0U && ((tailroom < dataSize) || (tailroom == 0 && dataSize == 0)) && tailroom < Page::CHUNK_MAX_SIZE/10) { + if(chunkCount == 0U && ((tailroom < dataSize) || (tailroom == 0 && dataSize == 0)) && tailroom < Page::CHUNK_MAX_SIZE/10) { /** This is the first chunk and tailroom is too small ***/ - if (page.state() != Page::PageState::FULL) { + if(page.state() != Page::PageState::FULL) { err = page.markFull(); - if (err != ESP_OK) { + if(err != ESP_OK) { return err; } } err = mPageManager.requestNewPage(); - if (err != ESP_OK) { + if(err != ESP_OK) { return err; } else if(getCurrentPage().getVarDataTailroom() == tailroom) { /* We got the same page or we are not improving.*/ @@ -307,7 +307,7 @@ esp_err_t Storage::writeMultiPageBlob(uint8_t nsIndex, const char* key, const vo } else { continue; } - } else if (!tailroom) { + } else if(!tailroom) { err = ESP_ERR_NVS_NOT_ENOUGH_SPACE; break; } @@ -322,32 +322,32 @@ esp_err_t Storage::writeMultiPageBlob(uint8_t nsIndex, const char* key, const vo static_cast (data) + offset, chunkSize, static_cast (chunkStart) + chunkCount); chunkCount++; - if (err != ESP_OK) { + if(err != ESP_OK) { NVS_ASSERT_OR_RETURN(err != ESP_ERR_NVS_PAGE_FULL, err); break; } else { UsedPageNode* node = new (std::nothrow) UsedPageNode(); - if (!node) { + if(!node) { err = ESP_ERR_NO_MEM; break; } node->mPage = &page; usedPages.push_back(node); - if (remainingSize || (tailroom - chunkSize) < Page::ENTRY_SIZE) { - if (page.state() != Page::PageState::FULL) { + if(remainingSize || (tailroom - chunkSize) < Page::ENTRY_SIZE) { + if(page.state() != Page::PageState::FULL) { err = page.markFull(); - if (err != ESP_OK) { + if(err != ESP_OK) { break; } } err = mPageManager.requestNewPage(); - if (err != ESP_OK) { + if(err != ESP_OK) { break; } } } offset += chunkSize; - if (!remainingSize) { + if(!remainingSize) { /* All pages are stored. Now store the index.*/ Item item; std::fill_n(item.data, sizeof(item.data), 0xff); @@ -359,12 +359,12 @@ esp_err_t Storage::writeMultiPageBlob(uint8_t nsIndex, const char* key, const vo NVS_ASSERT_OR_RETURN(err != ESP_ERR_NVS_PAGE_FULL, err); break; } - } while (1); + } while(1); - if (err != ESP_OK) { + if(err != ESP_OK) { /* Anything failed, then we should erase all the written chunks*/ int ii=0; - for (auto it = std::begin(usedPages); it != std::end(usedPages); it++) { + for(auto it = std::begin(usedPages); it != std::end(usedPages); it++) { it->mPage->eraseItem(nsIndex, ItemType::BLOB_DATA, key, ii++); } } @@ -372,200 +372,264 @@ esp_err_t Storage::writeMultiPageBlob(uint8_t nsIndex, const char* key, const vo return err; } +// datatype BLOB is written as BLOB_INDEX and BLOB_DATA and is searched for previous value as BLOB_INDEX and/or BLOB +// datatype BLOB_IDX and BLOB_DATA are not supported as input parameters, the layer above should always use BLOB esp_err_t Storage::writeItem(uint8_t nsIndex, ItemType datatype, const char* key, const void* data, size_t dataSize) { - if (mState != StorageState::ACTIVE) { + if(mState != StorageState::ACTIVE) { return ESP_ERR_NVS_NOT_INITIALIZED; } + // pointer to the page where the existing item was found Page* findPage = nullptr; + // page sequence number helping to detect whether the page with old value was relocated during the new write + uint32_t findPageSeqNumber = UINT32_MAX; + + // indicates the datatype representation match between the old value and the new one bool matchedTypePageFound = false; + + // contains the item with the old value, if found Item item; - esp_err_t err; - if (datatype == ItemType::BLOB) { + esp_err_t err = ESP_OK; + + // Try to find existing item with the same key and namespace index + // We are performing the findItem with datatype specified (it is not ANY) to ensure the hash list lookup is done. + if(datatype == ItemType::BLOB) { + // Specific lookup if performed for input datatype BLOB. The searched datatype for exact match is BLOB_INDEX. + // The BLOB_INDEX is used to store the metadata of the (originally typed) BLOB data in current V2 implementation. err = findItem(nsIndex, ItemType::BLOB_IDX, key, findPage, item); - if(err == ESP_OK) { - matchedTypePageFound = true; - } - } else { -#ifdef CONFIG_NVS_LEGACY_DUP_KEYS_COMPATIBILITY - err = findItem(nsIndex, datatype, key, findPage, item); if(err == ESP_OK && findPage != nullptr) { matchedTypePageFound = true; } -#else - err = findItem(nsIndex, ItemType::ANY, key, findPage, item); - if(err == ESP_OK && datatype == item.datatype) { + } else { + // Handle all other data types than BLOB + err = findItem(nsIndex, datatype, key, findPage, item); + if(err == ESP_OK && findPage != nullptr) { matchedTypePageFound = true; + + // keep the sequence number of the page where the item was found for later check of relocation + err = findPage->getSeqNumber(findPageSeqNumber); + if(err != ESP_OK) { + return err; + } } -#endif } - if (err != ESP_OK && err != ESP_ERR_NVS_NOT_FOUND) { +#ifndef CONFIG_NVS_LEGACY_DUP_KEYS_COMPATIBILITY + // If the item was not found, try to find it with any other datatype. Omit the BLOB_DATA and handle BLOB with respect + // to the V1 and V2 representation. + const ItemType dataTypes[] = { + ItemType::U8, + ItemType::U16, + ItemType::U32, + ItemType::U64, + ItemType::I8, + ItemType::I16, + ItemType::I32, + ItemType::I64, + ItemType::SZ, + ItemType::BLOB_IDX, + ItemType::BLOB + }; + + if(findPage == nullptr) { + // Iterate over potential data types to allow findItem() search using the hash list instead of bruteforce search. + for(const auto& currType : dataTypes) { + // Skip search for BLOB_IDX if the requested datatype is BLOB. BLOB_IDX was already searched above. + if(datatype == ItemType::BLOB && currType == ItemType::BLOB_IDX) continue; + + // Skip search if requested datatype is not BLOB and the current datatype is equal to the requested one. This was already searched above. + if(datatype != ItemType::BLOB && currType == datatype) continue; + + err = findItem(nsIndex, currType, key, findPage, item); + if(err == ESP_OK && findPage != nullptr) { + // keep the sequence number of the page where the item was found for later check of relocation + err = findPage->getSeqNumber(findPageSeqNumber); + if(err != ESP_OK) { + return err; + } + // item was found with the same key and namespace index but data type is different + matchedTypePageFound = false; + break; + } + } + } +#endif + + // Here the findPage is either nullptr or points to the page where the item was found. + // The matchedTypePageFound is true if the old value item was found and its datatype representation matches the new one. + // This flag is used to determine if the item should be checked for same value. + if(err != ESP_OK && err != ESP_ERR_NVS_NOT_FOUND) { return err; } - if (datatype == ItemType::BLOB) { + // Handle value update + if(datatype == ItemType::BLOB) { VerOffset prevStart, nextStart; prevStart = nextStart = VerOffset::VER_0_OFFSET; - if (matchedTypePageFound) { - // Do a sanity check that the item in question is actually being modified. + if(matchedTypePageFound) { + // Do a check that the item in question is actually being modified. // If it isn't, it is cheaper to purposefully not write out new data. // since it may invoke an erasure of flash. - if (cmpMultiPageBlob(nsIndex, key, data, dataSize) == ESP_OK) { + if(cmpMultiPageBlob(nsIndex, key, data, dataSize) == ESP_OK) { return ESP_OK; } - if (findPage->state() == Page::PageState::UNINITIALIZED || - findPage->state() == Page::PageState::INVALID) { - err = findItem(nsIndex, datatype, key, findPage, item); - if (err != ESP_OK) { - return err; - } - } - /* Get the version of the previous index with same */ + // Get the version of the previous index with same prevStart = item.blobIndex.chunkStart; NVS_ASSERT_OR_RETURN(prevStart == VerOffset::VER_0_OFFSET || prevStart == VerOffset::VER_1_OFFSET, ESP_FAIL); - - /* Toggle the version by changing the offset */ + // Toggle the version by changing the offset nextStart = (prevStart == VerOffset::VER_1_OFFSET) ? VerOffset::VER_0_OFFSET : VerOffset::VER_1_OFFSET; } - /* Write the blob with new version*/ + // Write the blob with new version err = writeMultiPageBlob(nsIndex, key, data, dataSize, nextStart); - if (err == ESP_ERR_NVS_PAGE_FULL) { + if(err == ESP_ERR_NVS_PAGE_FULL) { return ESP_ERR_NVS_NOT_ENOUGH_SPACE; } - if (err != ESP_OK) { + + if(err != ESP_OK) { return err; } - - if (matchedTypePageFound) { - /* Erase the blob with earlier version*/ - err = eraseMultiPageBlob(nsIndex, key, prevStart); - - if (err == ESP_ERR_FLASH_OP_FAIL) { - return ESP_ERR_NVS_REMOVE_FAILED; - } - if (err != ESP_OK) { - return err; - } - - findPage = nullptr; - } else { - /* Support for earlier versions where BLOBS were stored without index */ - err = findItem(nsIndex, datatype, key, findPage, item); - if (err != ESP_OK && err != ESP_ERR_NVS_NOT_FOUND) { - return err; - } - } } else { - // Do a sanity check that the item in question is actually being modified. + // Do a check that the item in question is actually being modified. // If it isn't, it is cheaper to purposefully not write out new data. // since it may invoke an erasure of flash. - if (matchedTypePageFound && + if(matchedTypePageFound && findPage->cmpItem(nsIndex, datatype, key, data, dataSize) == ESP_OK) { return ESP_OK; } Page& page = getCurrentPage(); err = page.writeItem(nsIndex, datatype, key, data, dataSize); - if (err == ESP_ERR_NVS_PAGE_FULL) { - if (page.state() != Page::PageState::FULL) { + if(err == ESP_ERR_NVS_PAGE_FULL) { + if(page.state() != Page::PageState::FULL) { err = page.markFull(); - if (err != ESP_OK) { + if(err != ESP_OK) { return err; } } err = mPageManager.requestNewPage(); - if (err != ESP_OK) { + if(err != ESP_OK) { return err; } err = getCurrentPage().writeItem(nsIndex, datatype, key, data, dataSize); - if (err == ESP_ERR_NVS_PAGE_FULL) { + if(err == ESP_ERR_NVS_PAGE_FULL) { return ESP_ERR_NVS_NOT_ENOUGH_SPACE; } - if (err != ESP_OK) { - return err; - } - } else if (err != ESP_OK) { + } + + if(err != ESP_OK) { return err; } } - if (findPage) { - if (findPage->state() == Page::PageState::UNINITIALIZED || - findPage->state() == Page::PageState::INVALID) { -#ifdef CONFIG_NVS_LEGACY_DUP_KEYS_COMPATIBILITY - err = findItem(nsIndex, datatype, key, findPage, item); -#else - err = findItem(nsIndex, ItemType::ANY, key, findPage, item); -#endif - if (err != ESP_OK) { + // Delete previous value + // Note: The old entry won't be deleted if the new value is the same as the old value - code won't reach here in that case. + + // If findPage is null then previous value was not present in NVS and nothig is to be deleted. + if(findPage == nullptr) { + return err; + } + + if(item.datatype == ItemType::BLOB_IDX) { + // If the item found was BLOB_INDEX, the eraseMultiPageBlob is used to erase the whole multi-page blob. + // It is not necessary to check the potential page relocation as the function will find the blob again anyway. + VerOffset prevStart = item.blobIndex.chunkStart; + err = eraseMultiPageBlob(nsIndex, key, prevStart); + + if(err == ESP_ERR_FLASH_OP_FAIL) { + return ESP_ERR_NVS_REMOVE_FAILED; + } + } else { + // For all other data types, we have to check the potential page relocation. + + // The findPage might have been relocated as a part of space reclaiming. + // First indication is the page state. It might become the "spare" page thus changing the state from FULL or ACTIVE. + bool wasRelocated = false; + + if( findPage->state() != Page::PageState::ACTIVE && + findPage->state() != Page::PageState::FULL) { + wasRelocated = true; + } + + // Other indication of the multi step relocation is the page sequence number. If the sequence number is different than page + // sequence number at the moment initial item was found, the page was relocated. + if(!wasRelocated) { + uint32_t newPageSeqNumber; + err = findPage->getSeqNumber(newPageSeqNumber); + if(err != ESP_OK) { + return err; + } + if(newPageSeqNumber != findPageSeqNumber) { + wasRelocated = true; + } + } + + if(wasRelocated) { + // The page was relocated. We have to find the old value again from the beginning. + // As the item was already found before relocation, we can use the exact datatype from item. + err = findItem(nsIndex, item.datatype, key, findPage, item); + if(err != ESP_OK) { return err; } } -#ifdef CONFIG_NVS_LEGACY_DUP_KEYS_COMPATIBILITY - err = findPage->eraseItem(nsIndex, datatype, key); -#else - err = findPage->eraseItem(nsIndex, ItemType::ANY, key); -#endif - if (err == ESP_ERR_FLASH_OP_FAIL) { + // Page containing the old value is now refreshed. We can erase the old value. + err = findPage->eraseItem(nsIndex, item.datatype, key); + if(err == ESP_ERR_FLASH_OP_FAIL) { return ESP_ERR_NVS_REMOVE_FAILED; } - if (err != ESP_OK) { - return err; - } } + #ifdef DEBUG_STORAGE debugCheck(); #endif - return ESP_OK; + return err; } esp_err_t Storage::createOrOpenNamespace(const char* nsName, bool canCreate, uint8_t& nsIndex) { - if (mState != StorageState::ACTIVE) { + if(mState != StorageState::ACTIVE) { return ESP_ERR_NVS_NOT_INITIALIZED; } auto it = std::find_if(mNamespaces.begin(), mNamespaces.end(), [=] (const NamespaceEntry& e) -> bool { return strncmp(nsName, e.mName, sizeof(e.mName) - 1) == 0; }); - if (it == std::end(mNamespaces)) { - if (!canCreate) { + if(it == std::end(mNamespaces)) { + if(!canCreate) { return ESP_ERR_NVS_NOT_FOUND; } uint8_t ns; bool ns_state; - for (ns = 1; ns < 255; ++ns) { - if (mNamespaceUsage.get(ns, &ns_state) != ESP_OK) { + for(ns = 1; ns < 255; ++ns) { + if(mNamespaceUsage.get(ns, &ns_state) != ESP_OK) { return ESP_FAIL; } - if (!ns_state) { + if(!ns_state) { break; } } - if (ns == 255) { + if(ns == 255) { return ESP_ERR_NVS_NOT_ENOUGH_SPACE; } auto err = writeItem(Page::NS_INDEX, ItemType::U8, nsName, &ns, sizeof(ns)); - if (err != ESP_OK) { + if(err != ESP_OK) { return err; } - if (mNamespaceUsage.set(ns, true) != ESP_OK) { + if(mNamespaceUsage.set(ns, true) != ESP_OK) { return ESP_FAIL; } nsIndex = ns; NamespaceEntry* entry = new (std::nothrow) NamespaceEntry; - if (!entry) { + if(!entry) { return ESP_ERR_NO_MEM; } @@ -587,7 +651,7 @@ esp_err_t Storage::readMultiPageBlob(uint8_t nsIndex, const char* key, void* dat /* First read the blob index */ auto err = findItem(nsIndex, ItemType::BLOB_IDX, key, findPage, item); - if (err != ESP_OK) { + if(err != ESP_OK) { return err; } @@ -598,21 +662,21 @@ esp_err_t Storage::readMultiPageBlob(uint8_t nsIndex, const char* key, void* dat NVS_ASSERT_OR_RETURN(dataSize == item.blobIndex.dataSize, ESP_FAIL); /* Now read corresponding chunks */ - for (uint8_t chunkNum = 0; chunkNum < chunkCount; chunkNum++) { + for(uint8_t chunkNum = 0; chunkNum < chunkCount; chunkNum++) { err = findItem(nsIndex, ItemType::BLOB_DATA, key, findPage, item, static_cast (chunkStart) + chunkNum); - if (err != ESP_OK) { - if (err == ESP_ERR_NVS_NOT_FOUND) { + if(err != ESP_OK) { + if(err == ESP_ERR_NVS_NOT_FOUND) { break; } return err; } - if (item.varLength.dataSize > dataSize - offset) { + if(item.varLength.dataSize > dataSize - offset) { /* The size of the entry in the index is inconsistent with the sum of the sizes of chunks */ err = ESP_ERR_NVS_INVALID_LENGTH; break; } err = findPage->readItem(nsIndex, ItemType::BLOB_DATA, key, static_cast(data) + offset, item.varLength.dataSize, static_cast (chunkStart) + chunkNum); - if (err != ESP_OK) { + if(err != ESP_OK) { return err; } NVS_ASSERT_OR_RETURN(static_cast (chunkStart) + chunkNum == item.chunkIndex, ESP_FAIL); @@ -620,7 +684,7 @@ esp_err_t Storage::readMultiPageBlob(uint8_t nsIndex, const char* key, void* dat offset += item.varLength.dataSize; } - if (err == ESP_ERR_NVS_NOT_FOUND || err == ESP_ERR_NVS_INVALID_LENGTH) { + if(err == ESP_ERR_NVS_NOT_FOUND || err == ESP_ERR_NVS_INVALID_LENGTH) { // cleanup if a chunk is not found or the size is inconsistent eraseMultiPageBlob(nsIndex, key); } @@ -637,7 +701,7 @@ esp_err_t Storage::cmpMultiPageBlob(uint8_t nsIndex, const char* key, const void /* First read the blob index */ auto err = findItem(nsIndex, ItemType::BLOB_IDX, key, findPage, item); - if (err != ESP_OK) { + if(err != ESP_OK) { return err; } @@ -646,21 +710,21 @@ esp_err_t Storage::cmpMultiPageBlob(uint8_t nsIndex, const char* key, const void size_t readSize = item.blobIndex.dataSize; size_t offset = 0; - if (dataSize != readSize) { + if(dataSize != readSize) { return ESP_ERR_NVS_CONTENT_DIFFERS; } /* Now read corresponding chunks */ - for (uint8_t chunkNum = 0; chunkNum < chunkCount; chunkNum++) { + for(uint8_t chunkNum = 0; chunkNum < chunkCount; chunkNum++) { err = findItem(nsIndex, ItemType::BLOB_DATA, key, findPage, item, static_cast (chunkStart) + chunkNum); - if (err != ESP_OK) { - if (err == ESP_ERR_NVS_NOT_FOUND) { + if(err != ESP_OK) { + if(err == ESP_ERR_NVS_NOT_FOUND) { break; } return err; } err = findPage->cmpItem(nsIndex, ItemType::BLOB_DATA, key, static_cast(data) + offset, item.varLength.dataSize, static_cast (chunkStart) + chunkNum); - if (err != ESP_OK) { + if(err != ESP_OK) { return err; } NVS_ASSERT_OR_RETURN(static_cast (chunkStart) + chunkNum == item.chunkIndex, ESP_FAIL); @@ -674,21 +738,21 @@ esp_err_t Storage::cmpMultiPageBlob(uint8_t nsIndex, const char* key, const void esp_err_t Storage::readItem(uint8_t nsIndex, ItemType datatype, const char* key, void* data, size_t dataSize) { - if (mState != StorageState::ACTIVE) { + if(mState != StorageState::ACTIVE) { return ESP_ERR_NVS_NOT_INITIALIZED; } Item item; Page* findPage = nullptr; - if (datatype == ItemType::BLOB) { + if(datatype == ItemType::BLOB) { auto err = readMultiPageBlob(nsIndex, key, data, dataSize); - if (err != ESP_ERR_NVS_NOT_FOUND) { + if(err != ESP_ERR_NVS_NOT_FOUND) { return err; } // else check if the blob is stored with earlier version format without index } auto err = findItem(nsIndex, datatype, key, findPage, item); - if (err != ESP_OK) { + if(err != ESP_OK) { return err; } return findPage->readItem(nsIndex, datatype, key, data, dataSize); @@ -697,19 +761,19 @@ esp_err_t Storage::readItem(uint8_t nsIndex, ItemType datatype, const char* key, esp_err_t Storage::eraseMultiPageBlob(uint8_t nsIndex, const char* key, VerOffset chunkStart) { - if (mState != StorageState::ACTIVE) { + if(mState != StorageState::ACTIVE) { return ESP_ERR_NVS_NOT_INITIALIZED; } Item item; Page* findPage = nullptr; auto err = findItem(nsIndex, ItemType::BLOB_IDX, key, findPage, item, Page::CHUNK_ANY, chunkStart); - if (err != ESP_OK) { + if(err != ESP_OK) { return err; } // Erase the index first and make children blobs orphan err = findPage->eraseItem(nsIndex, ItemType::BLOB_IDX, key, Page::CHUNK_ANY, chunkStart); - if (err != ESP_OK) { + if(err != ESP_OK) { return err; } @@ -719,12 +783,12 @@ esp_err_t Storage::eraseMultiPageBlob(uint8_t nsIndex, const char* key, VerOffse if(chunkStart == VerOffset::VER_ANY) { err = findItem(nsIndex, ItemType::BLOB_IDX, key, findPage, item, Page::CHUNK_ANY, chunkStart); - if (err == ESP_OK) { + if(err == ESP_OK) { err = findPage->eraseItem(nsIndex, ItemType::BLOB_IDX, key, Page::CHUNK_ANY, chunkStart); - if (err != ESP_OK) { + if(err != ESP_OK) { return err; } - } else if (err != ESP_ERR_NVS_NOT_FOUND) { + } else if(err != ESP_ERR_NVS_NOT_FOUND) { return err; } } @@ -735,17 +799,17 @@ esp_err_t Storage::eraseMultiPageBlob(uint8_t nsIndex, const char* key, VerOffse if(chunkStart == VerOffset::VER_0_OFFSET) { maxChunkIndex = (uint8_t) VerOffset::VER_1_OFFSET; - } else if (chunkStart == VerOffset::VER_1_OFFSET) { + } else if(chunkStart == VerOffset::VER_1_OFFSET) { minChunkIndex = (uint8_t) VerOffset::VER_1_OFFSET; } - for (auto it = std::begin(mPageManager); it != std::end(mPageManager); ++it) { + for(auto it = std::begin(mPageManager); it != std::end(mPageManager); ++it) { size_t itemIndex = 0; do { err = it->findItem(nsIndex, ItemType::BLOB_DATA, key, itemIndex, item); - if (err == ESP_ERR_NVS_NOT_FOUND) { + if(err == ESP_ERR_NVS_NOT_FOUND) { break; - } else if (err == ESP_OK) { + } else if(err == ESP_OK) { // check if item.chunkIndex is within the version range indicated by chunkStart, if so, delete it if((item.chunkIndex >= minChunkIndex) && (item.chunkIndex < maxChunkIndex)) { err = it->eraseEntryAndSpan(itemIndex); @@ -757,7 +821,7 @@ esp_err_t Storage::eraseMultiPageBlob(uint8_t nsIndex, const char* key, VerOffse if(err != ESP_OK) { return err; } - } while (err == ESP_OK && itemIndex < Page::ENTRY_COUNT); + } while(err == ESP_OK && itemIndex < Page::ENTRY_COUNT); } return ESP_OK; @@ -765,22 +829,22 @@ esp_err_t Storage::eraseMultiPageBlob(uint8_t nsIndex, const char* key, VerOffse esp_err_t Storage::eraseItem(uint8_t nsIndex, ItemType datatype, const char* key) { - if (mState != StorageState::ACTIVE) { + if(mState != StorageState::ACTIVE) { return ESP_ERR_NVS_NOT_INITIALIZED; } - if (datatype == ItemType::BLOB) { + if(datatype == ItemType::BLOB) { return eraseMultiPageBlob(nsIndex, key); } Item item; Page* findPage = nullptr; auto err = findItem(nsIndex, datatype, key, findPage, item); - if (err != ESP_OK) { + if(err != ESP_OK) { return err; } - if (item.datatype == ItemType::BLOB_DATA || item.datatype == ItemType::BLOB_IDX) { + if(item.datatype == ItemType::BLOB_DATA || item.datatype == ItemType::BLOB_IDX) { return eraseMultiPageBlob(nsIndex, key); } @@ -789,17 +853,17 @@ esp_err_t Storage::eraseItem(uint8_t nsIndex, ItemType datatype, const char* key esp_err_t Storage::eraseNamespace(uint8_t nsIndex) { - if (mState != StorageState::ACTIVE) { + if(mState != StorageState::ACTIVE) { return ESP_ERR_NVS_NOT_INITIALIZED; } - for (auto it = std::begin(mPageManager); it != std::end(mPageManager); ++it) { - while (true) { + for(auto it = std::begin(mPageManager); it != std::end(mPageManager); ++it) { + while(true) { auto err = it->eraseItem(nsIndex, ItemType::ANY, nullptr); - if (err == ESP_ERR_NVS_NOT_FOUND) { + if(err == ESP_ERR_NVS_NOT_FOUND) { break; } - else if (err != ESP_OK) { + else if(err != ESP_OK) { return err; } } @@ -810,14 +874,14 @@ esp_err_t Storage::eraseNamespace(uint8_t nsIndex) esp_err_t Storage::findKey(const uint8_t nsIndex, const char* key, ItemType* datatype) { - if (mState != StorageState::ACTIVE) { + if(mState != StorageState::ACTIVE) { return ESP_ERR_NVS_NOT_INITIALIZED; } Item item; Page* findPage = nullptr; auto err = findItem(nsIndex, ItemType::ANY, key, findPage, item); - if (err != ESP_OK) { + if(err != ESP_OK) { return err; } @@ -830,19 +894,19 @@ esp_err_t Storage::findKey(const uint8_t nsIndex, const char* key, ItemType* dat esp_err_t Storage::getItemDataSize(uint8_t nsIndex, ItemType datatype, const char* key, size_t& dataSize) { - if (mState != StorageState::ACTIVE) { + if(mState != StorageState::ACTIVE) { return ESP_ERR_NVS_NOT_INITIALIZED; } Item item; Page* findPage = nullptr; auto err = findItem(nsIndex, datatype, key, findPage, item); - if (err != ESP_OK) { - if (datatype != ItemType::BLOB) { + if(err != ESP_OK) { + if(datatype != ItemType::BLOB) { return err; } err = findItem(nsIndex, ItemType::BLOB_IDX, key, findPage, item); - if (err != ESP_OK) { + if(err != ESP_OK) { return err; } dataSize = item.blobIndex.dataSize; @@ -855,7 +919,7 @@ esp_err_t Storage::getItemDataSize(uint8_t nsIndex, ItemType datatype, const cha void Storage::debugDump() { - for (auto p = mPageManager.begin(); p != mPageManager.end(); ++p) { + for(auto p = mPageManager.begin(); p != mPageManager.end(); ++p) { p->debugDump(); } } @@ -865,15 +929,15 @@ void Storage::debugCheck() { std::map keys; - for (auto p = mPageManager.begin(); p != mPageManager.end(); ++p) { + for(auto p = mPageManager.begin(); p != mPageManager.end(); ++p) { size_t itemIndex = 0; size_t usedCount = 0; Item item; - while (p->findItem(Page::NS_ANY, ItemType::ANY, nullptr, itemIndex, item) == ESP_OK) { + while(p->findItem(Page::NS_ANY, ItemType::ANY, nullptr, itemIndex, item) == ESP_OK) { std::stringstream keyrepr; keyrepr << static_cast(item.nsIndex) << "_" << static_cast(item.datatype) << "_" << item.key <<"_"<(item.chunkIndex); std::string keystr = keyrepr.str(); - if (keys.find(keystr) != std::end(keys)) { + if(keys.find(keystr) != std::end(keys)) { printf("Duplicate key: %s\n", keystr.c_str()); debugDump(); assert(0); @@ -897,24 +961,24 @@ esp_err_t Storage::calcEntriesInNamespace(uint8_t nsIndex, size_t& usedEntries) { usedEntries = 0; - if (mState != StorageState::ACTIVE) { + if(mState != StorageState::ACTIVE) { return ESP_ERR_NVS_NOT_INITIALIZED; } - for (auto it = std::begin(mPageManager); it != std::end(mPageManager); ++it) { + for(auto it = std::begin(mPageManager); it != std::end(mPageManager); ++it) { size_t itemIndex = 0; Item item; - while (true) { + while(true) { auto err = it->findItem(nsIndex, ItemType::ANY, nullptr, itemIndex, item); - if (err == ESP_ERR_NVS_NOT_FOUND) { + if(err == ESP_ERR_NVS_NOT_FOUND) { break; } - else if (err != ESP_OK) { + else if(err != ESP_OK) { return err; } usedEntries += item.span; itemIndex += item.span; - if (itemIndex >= it->ENTRY_COUNT) break; + if(itemIndex >= it->ENTRY_COUNT) break; } } return ESP_OK; @@ -926,7 +990,7 @@ void Storage::fillEntryInfo(Item &item, nvs_entry_info_t &info) strncpy(info.key, item.key, sizeof(info.key) - 1); info.key[sizeof(info.key) - 1] = '\0'; - for (auto &name : mNamespaces) { + for(auto &name : mNamespaces) { if(item.nsIndex == name.mIndex) { strlcpy(info.namespace_name, name.mName, sizeof(info.namespace_name)); break; @@ -940,7 +1004,7 @@ bool Storage::findEntry(nvs_opaque_iterator_t* it, const char* namespace_name) it->nsIndex = Page::NS_ANY; it->page = mPageManager.begin(); - if (namespace_name != nullptr) { + if(namespace_name != nullptr) { if(createOrOpenNamespace(namespace_name, false, it->nsIndex) != ESP_OK) { return false; } @@ -977,7 +1041,7 @@ bool Storage::nextEntry(nvs_opaque_iterator_t* it) Item item; esp_err_t err; - for (auto page = it->page; page != mPageManager.end(); ++page) { + for(auto page = it->page; page != mPageManager.end(); ++page) { do { err = page->findItem(it->nsIndex, (ItemType)it->type, nullptr, it->entryIndex, item); it->entryIndex += item.span; @@ -986,7 +1050,7 @@ bool Storage::nextEntry(nvs_opaque_iterator_t* it) it->page = page; return true; } - } while (err != ESP_ERR_NVS_NOT_FOUND); + } while(err != ESP_ERR_NVS_NOT_FOUND); it->entryIndex = 0; }