From 3ce433cfd1e379aada08b91aa544d79ee3f6ab84 Mon Sep 17 00:00:00 2001 From: Ivan Grokhotkov Date: Mon, 22 Aug 2016 10:30:35 +0800 Subject: [PATCH 1/9] components/nvs: make some methods const --- components/nvs_flash/src/compressed_enum_table.hpp | 2 +- components/nvs_flash/src/nvs_page.cpp | 4 ++-- components/nvs_flash/src/nvs_page.hpp | 6 +++--- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/components/nvs_flash/src/compressed_enum_table.hpp b/components/nvs_flash/src/compressed_enum_table.hpp index 8ee95d9f99..319d86a45c 100644 --- a/components/nvs_flash/src/compressed_enum_table.hpp +++ b/components/nvs_flash/src/compressed_enum_table.hpp @@ -33,7 +33,7 @@ public: return mData; } - Tenum get(size_t index) + Tenum get(size_t index) const { assert(index >= 0 && index < Nitems); size_t wordIndex = index / ITEMS_PER_WORD; diff --git a/components/nvs_flash/src/nvs_page.cpp b/components/nvs_flash/src/nvs_page.cpp index de325a170c..0633f5d946 100644 --- a/components/nvs_flash/src/nvs_page.cpp +++ b/components/nvs_flash/src/nvs_page.cpp @@ -536,7 +536,7 @@ esp_err_t Page::alterPageState(PageState state) return ESP_OK; } -esp_err_t Page::readEntry(size_t index, Item& dst) +esp_err_t Page::readEntry(size_t index, Item& dst) const { auto rc = spi_flash_read(getEntryAddress(index), reinterpret_cast(&dst), sizeof(dst)); if (rc != ESP_OK) { @@ -658,7 +658,7 @@ void Page::invalidateCache() mFindInfo = CachedFindInfo(); } -void Page::debugDump() +void Page::debugDump() const { printf("state=%x addr=%x seq=%d\nfirstUsed=%d nextFree=%d used=%d erased=%d\n", mState, mBaseAddress, mSeqNumber, static_cast(mFirstUsedEntry), static_cast(mNextFreeEntry), mUsedEntryCount, mErasedEntryCount); size_t skip = 0; diff --git a/components/nvs_flash/src/nvs_page.hpp b/components/nvs_flash/src/nvs_page.hpp index f0c69c4246..a962aa3bfc 100644 --- a/components/nvs_flash/src/nvs_page.hpp +++ b/components/nvs_flash/src/nvs_page.hpp @@ -162,7 +162,7 @@ public: void invalidateCache(); - void debugDump(); + void debugDump() const; protected: @@ -195,7 +195,7 @@ protected: esp_err_t alterPageState(PageState state); - esp_err_t readEntry(size_t index, Item& dst); + esp_err_t readEntry(size_t index, Item& dst) const; esp_err_t writeEntry(const Item& item); @@ -210,7 +210,7 @@ protected: return static_cast(type) & 0x0f; } - uint32_t getEntryAddress(size_t entry) + uint32_t getEntryAddress(size_t entry) const { assert(entry < ENTRY_COUNT); return mBaseAddress + ENTRY_DATA_OFFSET + static_cast(entry) * ENTRY_SIZE; From f04c89412387ad5ebc824ba0a02030e8071bc9d3 Mon Sep 17 00:00:00 2001 From: Ivan Grokhotkov Date: Mon, 22 Aug 2016 11:46:44 +0800 Subject: [PATCH 2/9] components/nvs: add debugging facilities and runtime checks Extra runtime sanity checks run when compiled for the host (i.e. with no ESP_PLATFORM define) --- components/nvs_flash/src/nvs_api.cpp | 6 ++ components/nvs_flash/src/nvs_page.cpp | 1 + components/nvs_flash/src/nvs_pagemanager.cpp | 3 + components/nvs_flash/src/nvs_storage.cpp | 62 +++++++++++++++++++- components/nvs_flash/src/nvs_storage.hpp | 3 + 5 files changed, 72 insertions(+), 3 deletions(-) diff --git a/components/nvs_flash/src/nvs_api.cpp b/components/nvs_flash/src/nvs_api.cpp index 655ed24f87..041dc9e46e 100644 --- a/components/nvs_flash/src/nvs_api.cpp +++ b/components/nvs_flash/src/nvs_api.cpp @@ -45,6 +45,12 @@ static intrusive_list s_nvs_handles; static uint32_t s_nvs_next_handle = 1; static nvs::Storage s_nvs_storage; +extern "C" void nvs_dump() +{ + Lock lock; + s_nvs_storage.debugDump(); +} + extern "C" esp_err_t nvs_flash_init(uint32_t baseSector, uint32_t sectorCount) { Lock::init(); diff --git a/components/nvs_flash/src/nvs_page.cpp b/components/nvs_flash/src/nvs_page.cpp index 0633f5d946..903adfaedf 100644 --- a/components/nvs_flash/src/nvs_page.cpp +++ b/components/nvs_flash/src/nvs_page.cpp @@ -17,6 +17,7 @@ #else #include "crc.h" #endif +#include namespace nvs diff --git a/components/nvs_flash/src/nvs_pagemanager.cpp b/components/nvs_flash/src/nvs_pagemanager.cpp index 8fd62430d5..bdf609aa94 100644 --- a/components/nvs_flash/src/nvs_pagemanager.cpp +++ b/components/nvs_flash/src/nvs_pagemanager.cpp @@ -91,6 +91,7 @@ esp_err_t PageManager::requestNewPage() Page* newPage = &mPageList.back(); Page* erasedPage = maxErasedItemsPageIt; + size_t usedEntries = erasedPage->getUsedEntryCount(); err = erasedPage->markFreeing(); if (err != ESP_OK) { return err; @@ -108,6 +109,8 @@ esp_err_t PageManager::requestNewPage() if (err != ESP_OK) { return err; } + + assert(usedEntries == newPage->getUsedEntryCount()); mPageList.erase(maxErasedItemsPageIt); mFreePageList.push_back(erasedPage); diff --git a/components/nvs_flash/src/nvs_storage.cpp b/components/nvs_flash/src/nvs_storage.cpp index 510c52407a..32738fa298 100644 --- a/components/nvs_flash/src/nvs_storage.cpp +++ b/components/nvs_flash/src/nvs_storage.cpp @@ -13,6 +13,11 @@ // limitations under the License. #include "nvs_storage.hpp" +#ifndef ESP_PLATFORM +#include +#include +#endif + namespace nvs { @@ -56,6 +61,9 @@ esp_err_t Storage::init(uint32_t baseSector, uint32_t sectorCount) mNamespaceUsage.set(0, true); mNamespaceUsage.set(255, true); mState = StorageState::ACTIVE; +#ifndef ESP_PLATFORM + debugCheck(); +#endif return ESP_OK; } @@ -75,29 +83,46 @@ esp_err_t Storage::writeItem(uint8_t nsIndex, ItemType datatype, const char* key Page& page = getCurrentPage(); err = page.writeItem(nsIndex, datatype, key, data, dataSize); if (err == ESP_ERR_NVS_PAGE_FULL) { - page.markFull(); + if (page.state() != Page::PageState::FULL) { + err = page.markFull(); + if (err != ESP_OK) { + return err; + } + } err = mPageManager.requestNewPage(); if (err != ESP_OK) { return err; } err = getCurrentPage().writeItem(nsIndex, datatype, key, data, dataSize); + 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) { + return err; + } if (findPage) { - if (findPage->state() == Page::PageState::UNINITIALIZED) { + if (findPage->state() == Page::PageState::UNINITIALIZED || + findPage->state() == Page::PageState::INVALID) { auto err = findItem(nsIndex, datatype, key, findPage, item); assert(err == ESP_OK); } err = findPage->eraseItem(nsIndex, datatype, key); + if (err == ESP_ERR_FLASH_OP_FAIL) { + return ESP_ERR_NVS_REMOVE_FAILED; + } if (err != ESP_OK) { return err; } } - +#ifndef ESP_PLATFORM + debugCheck(); +#endif return ESP_OK; } @@ -192,4 +217,35 @@ esp_err_t Storage::getItemDataSize(uint8_t nsIndex, ItemType datatype, const cha return ESP_OK; } +void Storage::debugDump() +{ + for (auto p = mPageManager.begin(); p != mPageManager.end(); ++p) { + p->debugDump(); + } +} + +#ifndef ESP_PLATFORM +void Storage::debugCheck() +{ + std::map keys; + + for (auto p = mPageManager.begin(); p != mPageManager.end(); ++p) { + size_t itemIndex = 0; + Item item; + 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; + std::string keystr = keyrepr.str(); + if (keys.find(keystr) != std::end(keys)) { + printf("Duplicate key: %s\n", keystr.c_str()); + debugDump(); + assert(0); + } + keys.insert(std::make_pair(keystr, static_cast(p))); + itemIndex += item.span; + } + } +} +#endif //ESP_PLATFORM + } \ No newline at end of file diff --git a/components/nvs_flash/src/nvs_storage.hpp b/components/nvs_flash/src/nvs_storage.hpp index d61b219b26..3a05c32668 100644 --- a/components/nvs_flash/src/nvs_storage.hpp +++ b/components/nvs_flash/src/nvs_storage.hpp @@ -74,6 +74,9 @@ public: { return eraseItem(nsIndex, itemTypeOf(), key); } + + void debugDump(); + void debugCheck(); protected: From 35d50643bc2516e8030f16afc44474594bbcd9fa Mon Sep 17 00:00:00 2001 From: Ivan Grokhotkov Date: Mon, 22 Aug 2016 11:50:31 +0800 Subject: [PATCH 3/9] components/nvs: fix infinite loop when loading namespaces --- components/nvs_flash/src/nvs_storage.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/components/nvs_flash/src/nvs_storage.cpp b/components/nvs_flash/src/nvs_storage.cpp index 32738fa298..5cbc52bee3 100644 --- a/components/nvs_flash/src/nvs_storage.cpp +++ b/components/nvs_flash/src/nvs_storage.cpp @@ -44,6 +44,7 @@ esp_err_t Storage::init(uint32_t baseSector, uint32_t sectorCount) return err; } + // load namespaces list clearNamespaces(); std::fill_n(mNamespaceUsage.data(), mNamespaceUsage.byteSize() / 4, 0); for (auto it = mPageManager.begin(); it != mPageManager.end(); ++it) { @@ -56,6 +57,7 @@ esp_err_t Storage::init(uint32_t baseSector, uint32_t sectorCount) item.getValue(entry->mIndex); mNamespaces.push_back(entry); mNamespaceUsage.set(entry->mIndex, true); + itemIndex += item.span; } } mNamespaceUsage.set(0, true); From 7447d0860513dc8e87532e5958a8b29a08a818bf Mon Sep 17 00:00:00 2001 From: Ivan Grokhotkov Date: Mon, 22 Aug 2016 11:58:13 +0800 Subject: [PATCH 4/9] components/nvs: clear handles list on init, fix returning *length in nvs_get_{str,blob} --- components/nvs_flash/src/nvs_api.cpp | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/components/nvs_flash/src/nvs_api.cpp b/components/nvs_flash/src/nvs_api.cpp index 041dc9e46e..f9292e680e 100644 --- a/components/nvs_flash/src/nvs_api.cpp +++ b/components/nvs_flash/src/nvs_api.cpp @@ -56,6 +56,7 @@ extern "C" esp_err_t nvs_flash_init(uint32_t baseSector, uint32_t sectorCount) Lock::init(); Lock lock; NVS_DEBUGV("%s %d %d\r\n", __func__, baseSector, sectorCount); + s_nvs_handles.clear(); return s_nvs_storage.init(baseSector, sectorCount); } @@ -260,12 +261,15 @@ static esp_err_t nvs_get_str_or_blob(nvs_handle handle, nvs::ItemType type, cons return err; } - if (length != nullptr && out_value == nullptr) { + if (length == nullptr) { + return ESP_ERR_NVS_INVALID_LENGTH; + } + else if (out_value == nullptr) { *length = dataSize; return ESP_OK; } - - if (length == nullptr || *length < dataSize) { + else if (*length < dataSize) { + *length = dataSize; return ESP_ERR_NVS_INVALID_LENGTH; } From 7998b6ca2efb948cf6b06440a30f7450ced6feeb Mon Sep 17 00:00:00 2001 From: Ivan Grokhotkov Date: Mon, 22 Aug 2016 12:11:32 +0800 Subject: [PATCH 5/9] components/nvs: handle more cases where sudden power off may happen This commit fixes several issues with state handling in nvs::Page. It also adds extra consistency checks in nvs::PageManger initialization. These changes were verified with a new long-running test ("test recovery from sudden poweroff"). This test works by repeatedly performing same pseudorandom sequence of calls to nvs_ APIs. Each time it repeats the sequence, it introduces a failure into one of flash operations (write or erase). So if one iteration of this test needs, say, 25000 flash operations, then this test will run 25000 iterations, each time introducing the failure point at different location. --- components/nvs_flash/include/nvs.h | 5 + components/nvs_flash/src/nvs_page.cpp | 101 +++-- components/nvs_flash/src/nvs_page.hpp | 2 +- components/nvs_flash/src/nvs_pagemanager.cpp | 52 +++ .../nvs_flash/test/spi_flash_emulation.h | 14 + components/nvs_flash/test/test_nvs.cpp | 392 +++++++++++------- 6 files changed, 381 insertions(+), 185 deletions(-) diff --git a/components/nvs_flash/include/nvs.h b/components/nvs_flash/include/nvs.h index 2d0f27544c..6fff2dabf6 100644 --- a/components/nvs_flash/include/nvs.h +++ b/components/nvs_flash/include/nvs.h @@ -36,6 +36,7 @@ typedef uint32_t nvs_handle; #define ESP_ERR_NVS_NOT_ENOUGH_SPACE (ESP_ERR_NVS_BASE + 0x05) #define ESP_ERR_NVS_INVALID_NAME (ESP_ERR_NVS_BASE + 0x06) #define ESP_ERR_NVS_INVALID_HANDLE (ESP_ERR_NVS_BASE + 0x07) +#define ESP_ERR_NVS_REMOVE_FAILED (ESP_ERR_NVS_BASE + 0x08) #define ESP_ERR_NVS_KEY_TOO_LONG (ESP_ERR_NVS_BASE + 0x09) #define ESP_ERR_NVS_PAGE_FULL (ESP_ERR_NVS_BASE + 0x0a) #define ESP_ERR_NVS_INVALID_STATE (ESP_ERR_NVS_BASE + 0x0b) @@ -92,6 +93,10 @@ esp_err_t nvs_open(const char* name, nvs_open_mode open_mode, nvs_handle *out_ha * - ESP_ERR_NVS_INVALID_NAME if key name doesn't satisfy constraints * - ESP_ERR_NVS_NOT_ENOUGH_SPACE if there is not enough space in the * underlying storage to save the value + * - ESP_ERR_NVS_REMOVE_FAILED if the value wasn't updated because flash + * write operation has failed. The value was written however, and + * update will be finished after re-initialization of nvs, provided that + * flash operation doesn't fail again. */ esp_err_t nvs_set_i8 (nvs_handle handle, const char* key, int8_t value); esp_err_t nvs_set_u8 (nvs_handle handle, const char* key, uint8_t value); diff --git a/components/nvs_flash/src/nvs_page.cpp b/components/nvs_flash/src/nvs_page.cpp index 903adfaedf..918908dd6f 100644 --- a/components/nvs_flash/src/nvs_page.cpp +++ b/components/nvs_flash/src/nvs_page.cpp @@ -99,15 +99,12 @@ esp_err_t Page::writeEntry(const Item& item) return err; } - if (mNextFreeEntry == 0) { - mFirstUsedEntry = 0; + if (mFirstUsedEntry == INVALID_ENTRY) { + mFirstUsedEntry = mNextFreeEntry; } ++mUsedEntryCount; - - if (++mNextFreeEntry == ENTRY_COUNT) { - alterPageState(PageState::FULL); - } + ++mNextFreeEntry; return ESP_OK; } @@ -144,7 +141,7 @@ esp_err_t Page::writeItem(uint8_t nsIndex, ItemType datatype, const char* key, c // primitive types should fit into one entry assert(totalSize == ENTRY_SIZE || datatype == ItemType::BLOB || datatype == ItemType::SZ); - if (mNextFreeEntry + entriesCount > ENTRY_COUNT) { + if (mNextFreeEntry == INVALID_ENTRY || mNextFreeEntry + entriesCount > ENTRY_COUNT) { // page will not fit this amount of data return ESP_ERR_NVS_PAGE_FULL; } @@ -296,10 +293,14 @@ esp_err_t Page::eraseEntryAndSpan(size_t index) } else { span = item.span; for (ptrdiff_t i = index + span - 1; i >= static_cast(index); --i) { + if (mEntryTable.get(i) == EntryState::WRITTEN) { + --mUsedEntryCount; + } rc = alterEntryState(i, EntryState::ERASED); if (rc != ESP_OK) { return rc; } + ++mErasedEntryCount; } } } @@ -313,9 +314,10 @@ esp_err_t Page::eraseEntryAndSpan(size_t index) if (index == mFirstUsedEntry) { updateFirstUsedEntry(index, span); } - - mErasedEntryCount += span; - mUsedEntryCount -= span; + + if (index + span > mNextFreeEntry) { + mNextFreeEntry = index + span; + } return ESP_OK; } @@ -324,7 +326,11 @@ void Page::updateFirstUsedEntry(size_t index, size_t span) { assert(index == mFirstUsedEntry); mFirstUsedEntry = INVALID_ENTRY; - for (size_t i = index + span; i < mNextFreeEntry; ++i) { + size_t end = mNextFreeEntry; + if (end > ENTRY_COUNT) { + end = ENTRY_COUNT; + } + for (size_t i = index + span; i < end; ++i) { if (mEntryTable.get(i) == EntryState::WRITTEN) { mFirstUsedEntry = i; break; @@ -358,10 +364,6 @@ esp_err_t Page::moveItem(Page& other) if (err != ESP_OK) { return err; } - err = eraseEntry(mFirstUsedEntry); - if (err != ESP_OK) { - return err; - } size_t span = entry.span; size_t end = mFirstUsedEntry + span; @@ -374,6 +376,8 @@ esp_err_t Page::moveItem(Page& other) if (err != ESP_OK) { return err; } + } + for (size_t i = mFirstUsedEntry; i < end; ++i) { err = eraseEntry(i); if (err != ESP_OK) { return err; @@ -428,30 +432,39 @@ esp_err_t Page::mLoadEntryTable() // but before the entry state table was altered, the entry locacted via // entry state table may actually be half-written. // this is easy to check by reading EntryHeader (i.e. first word) - uint32_t entryAddress = mBaseAddress + ENTRY_DATA_OFFSET + - static_cast(mNextFreeEntry) * ENTRY_SIZE; - uint32_t header; - auto rc = spi_flash_read(entryAddress, &header, sizeof(header)); - if (rc != ESP_OK) { - mState = PageState::INVALID; - return rc; - } - if (header != 0xffffffff) { - auto err = alterEntryState(mNextFreeEntry, EntryState::ERASED); - if (err != ESP_OK) { + if (mNextFreeEntry != INVALID_ENTRY) { + uint32_t entryAddress = getEntryAddress(mNextFreeEntry); + uint32_t header; + auto rc = spi_flash_read(entryAddress, &header, sizeof(header)); + if (rc != ESP_OK) { mState = PageState::INVALID; - return err; + return rc; + } + if (header != 0xffffffff) { + auto err = alterEntryState(mNextFreeEntry, EntryState::ERASED); + if (err != ESP_OK) { + mState = PageState::INVALID; + return err; + } + ++mNextFreeEntry; } - ++mNextFreeEntry; } // check that all variable-length items are written or erased fully - for (size_t i = 0; i < mNextFreeEntry; ++i) { + Item item; + size_t lastItemIndex = INVALID_ENTRY; + size_t end = mNextFreeEntry; + if (end > ENTRY_COUNT) { + end = ENTRY_COUNT; + } + for (size_t i = 0; i < end; ++i) { if (mEntryTable.get(i) == EntryState::ERASED) { + lastItemIndex = INVALID_ENTRY; continue; } + + lastItemIndex = i; - Item item; auto err = readEntry(i, item); if (err != ESP_OK) { mState = PageState::INVALID; @@ -476,6 +489,7 @@ esp_err_t Page::mLoadEntryTable() for (size_t j = i; j < i + span; ++j) { if (mEntryTable.get(j) != EntryState::WRITTEN) { needErase = true; + lastItemIndex = INVALID_ENTRY; break; } } @@ -484,7 +498,21 @@ esp_err_t Page::mLoadEntryTable() } i += span - 1; } - + + // check that last item is not duplicate + if (lastItemIndex != INVALID_ENTRY) { + size_t findItemIndex = 0; + Item dupItem; + if (findItem(item.nsIndex, item.datatype, item.key, findItemIndex, dupItem) == ESP_OK) { + if (findItemIndex < lastItemIndex) { + auto err = eraseEntryAndSpan(findItemIndex); + if (err != ESP_OK) { + mState = PageState::INVALID; + return err; + } + } + } + } } return ESP_OK; @@ -551,6 +579,10 @@ esp_err_t Page::findItem(uint8_t nsIndex, ItemType datatype, const char* key, si if (mState == PageState::CORRUPT || mState == PageState::INVALID || mState == PageState::UNINITIALIZED) { return ESP_ERR_NVS_NOT_FOUND; } + + if (itemIndex >= ENTRY_COUNT) { + return ESP_ERR_NVS_NOT_FOUND; + } CachedFindInfo findInfo(nsIndex, datatype, key); if (mFindInfo == findInfo) { @@ -561,9 +593,14 @@ esp_err_t Page::findItem(uint8_t nsIndex, ItemType datatype, const char* key, si if (itemIndex > mFirstUsedEntry && itemIndex < ENTRY_COUNT) { start = itemIndex; } + + size_t end = mNextFreeEntry; + if (end > ENTRY_COUNT) { + end = ENTRY_COUNT; + } size_t next; - for (size_t i = start; i < mNextFreeEntry; i = next) { + for (size_t i = start; i < end; i = next) { next = i + 1; if (mEntryTable.get(i) != EntryState::WRITTEN) { continue; diff --git a/components/nvs_flash/src/nvs_page.hpp b/components/nvs_flash/src/nvs_page.hpp index a962aa3bfc..9afc6ff216 100644 --- a/components/nvs_flash/src/nvs_page.hpp +++ b/components/nvs_flash/src/nvs_page.hpp @@ -40,7 +40,7 @@ public: bool operator==(const CachedFindInfo& other) const { - return mKeyPtr == other.mKeyPtr && mType == other.mType && mNsIndex == other.mNsIndex; + return mKeyPtr != nullptr && mKeyPtr == other.mKeyPtr && mType == other.mType && mNsIndex == other.mNsIndex; } void setItemIndex(uint32_t index) diff --git a/components/nvs_flash/src/nvs_pagemanager.cpp b/components/nvs_flash/src/nvs_pagemanager.cpp index bdf609aa94..c0e904e35c 100644 --- a/components/nvs_flash/src/nvs_pagemanager.cpp +++ b/components/nvs_flash/src/nvs_pagemanager.cpp @@ -53,6 +53,58 @@ esp_err_t PageManager::load(uint32_t baseSector, uint32_t sectorCount) assert(mPageList.back().getSeqNumber(lastSeqNo) == ESP_OK); mSeqNumber = lastSeqNo + 1; } + + // if power went out after a new item for the given key was written, + // but before the old one was erased, we end up with a duplicate item + Page& lastPage = back(); + size_t lastItemIndex = SIZE_MAX; + Item item; + size_t itemIndex = 0; + while (lastPage.findItem(Page::NS_ANY, ItemType::ANY, nullptr, itemIndex, item) == ESP_OK) { + itemIndex += item.span; + lastItemIndex = itemIndex; + } + + if (lastItemIndex != SIZE_MAX) { + auto last = PageManager::TPageListIterator(&lastPage); + for (auto it = begin(); it != last; ++it) { + if (it->eraseItem(item.nsIndex, item.datatype, item.key) == ESP_OK) { + break; + } + } + } + + // check if power went out while page was being freed + for (auto it = begin(); it!= end(); ++it) { + if (it->state() == Page::PageState::FREEING) { + Page* newPage = &mPageList.back(); + if(newPage->state() != Page::PageState::ACTIVE) { + auto err = activatePage(); + if (err != ESP_OK) { + return err; + } + newPage = &mPageList.back(); + } + while (true) { + auto err = it->moveItem(*newPage); + if (err == ESP_ERR_NVS_NOT_FOUND) { + break; + } else if (err != ESP_OK) { + return err; + } + } + + auto err = it->erase(); + if (err != ESP_OK) { + return err; + } + + Page* p = static_cast(it); + mPageList.erase(it); + mFreePageList.push_back(p); + break; + } + } return ESP_OK; } diff --git a/components/nvs_flash/test/spi_flash_emulation.h b/components/nvs_flash/test/spi_flash_emulation.h index a321c61b37..ca1f9e27f5 100644 --- a/components/nvs_flash/test/spi_flash_emulation.h +++ b/components/nvs_flash/test/spi_flash_emulation.h @@ -73,6 +73,10 @@ public: dstAddr + size > mData.size() * 4) { return false; } + + if (mFailCountdown != SIZE_T_MAX && mFailCountdown-- == 0) { + return false; + } for (size_t i = 0; i < size / 4; ++i) { uint32_t sv = src[i]; @@ -103,6 +107,10 @@ public: WARN("invalid flash operation detected: erase sector=" << sectorNumber); return false; } + + if (mFailCountdown != SIZE_T_MAX && mFailCountdown-- == 0) { + return false; + } std::fill_n(begin(mData) + offset, SPI_FLASH_SEC_SIZE / 4, 0xffffffff); @@ -173,6 +181,10 @@ public: mLowerSectorBound = lowerSector; mUpperSectorBound = upperSector; } + + void failAfter(uint32_t count) { + mFailCountdown = count; + } protected: static size_t getReadOpTime(uint32_t bytes); @@ -190,6 +202,8 @@ protected: mutable size_t mTotalTime = 0; size_t mLowerSectorBound = 0; size_t mUpperSectorBound = 0; + + size_t mFailCountdown = SIZE_T_MAX; }; diff --git a/components/nvs_flash/test/test_nvs.cpp b/components/nvs_flash/test/test_nvs.cpp index 713513a03d..6e8f4124fc 100644 --- a/components/nvs_flash/test/test_nvs.cpp +++ b/components/nvs_flash/test/test_nvs.cpp @@ -153,9 +153,7 @@ TEST_CASE("when page is full, adding an element fails", "[nvs]") snprintf(name, sizeof(name), "i%ld", i); CHECK(page.writeItem(1, name, i) == ESP_OK); } - CHECK(page.state() == Page::PageState::FULL); CHECK(page.writeItem(1, "foo", 64UL) == ESP_ERR_NVS_PAGE_FULL); - CHECK(page.state() == Page::PageState::FULL); } TEST_CASE("page maintains its seq number") @@ -527,162 +525,194 @@ TEST_CASE("nvs api tests, starting with random data in flash", "[nvs][.][long]") } } -template -esp_err_t doRandomThings(nvs_handle handle, TGen gen, size_t count) { - - const char* keys[] = {"foo", "bar", "longkey_0123456", "another key", "param1", "param2", "param3", "param4", "param5"}; - const ItemType types[] = {ItemType::I32, ItemType::I32, ItemType::U64, ItemType::U64, ItemType::SZ, ItemType::SZ, ItemType::SZ, ItemType::SZ, ItemType::SZ}; +extern "C" void nvs_dump(); + +class RandomTest { + static const size_t nKeys = 9; int32_t v1 = 0, v2 = 0; uint64_t v3 = 0, v4 = 0; - const size_t strBufLen = 1024; + static const size_t strBufLen = 1024; char v5[strBufLen], v6[strBufLen], v7[strBufLen], v8[strBufLen], v9[strBufLen]; - - void* values[] = {&v1, &v2, &v3, &v4, &v5, &v6, &v7, &v8, &v9}; - - const size_t nKeys = sizeof(keys) / sizeof(keys[0]); - static_assert(nKeys == sizeof(types) / sizeof(types[0]), ""); - static_assert(nKeys == sizeof(values) / sizeof(values[0]), ""); - bool written[nKeys]; - std::fill_n(written, nKeys, false); - auto generateRandomString = [](char* dst, size_t size) { - size_t len = 0; - }; - - auto randomRead = [&](size_t index) -> esp_err_t { - switch (types[index]) { - case ItemType::I32: - { - int32_t val; - auto err = nvs_get_i32(handle, keys[index], &val); - if (err == ESP_ERR_FLASH_OP_FAIL) { - return err; - } - if (!written[index]) { - REQUIRE(err == ESP_ERR_NVS_NOT_FOUND); - } - else { - REQUIRE(val == *reinterpret_cast(values[index])); - } - break; - } - - case ItemType::U64: - { - uint64_t val; - auto err = nvs_get_u64(handle, keys[index], &val); - if (err == ESP_ERR_FLASH_OP_FAIL) { - return err; - } - if (!written[index]) { - REQUIRE(err == ESP_ERR_NVS_NOT_FOUND); - } - else { - REQUIRE(val == *reinterpret_cast(values[index])); - } - break; - } - - case ItemType::SZ: - { - char buf[strBufLen]; - size_t len = strBufLen; - auto err = nvs_get_str(handle, keys[index], buf, &len); - if (err == ESP_ERR_FLASH_OP_FAIL) { - return err; - } - if (!written[index]) { - REQUIRE(err == ESP_ERR_NVS_NOT_FOUND); - } - else { - REQUIRE(strncmp(buf, reinterpret_cast(values[index]), strBufLen)); - } - break; - } - - default: - assert(0); - } - return ESP_OK; - }; - - auto randomWrite = [&](size_t index) -> esp_err_t { - switch (types[index]) { - case ItemType::I32: - { - int32_t val = static_cast(gen()); - *reinterpret_cast(values[index]) = val; - - auto err = nvs_set_i32(handle, keys[index], val); - if (err == ESP_ERR_FLASH_OP_FAIL) { - return err; - } - REQUIRE(err == ESP_OK); - written[index] = true; - break; - } - - case ItemType::U64: - { - uint64_t val = static_cast(gen()); - *reinterpret_cast(values[index]) = val; - - auto err = nvs_set_u64(handle, keys[index], val); - if (err == ESP_ERR_FLASH_OP_FAIL) { - return err; - } - REQUIRE(err == ESP_OK); - written[index] = true; - break; - } - - case ItemType::SZ: - { - char buf[strBufLen]; - size_t len = strBufLen; - - size_t strLen = gen() % (strBufLen - 1); - std::generate_n(buf, strLen, [&]() -> char { - const char c = static_cast(gen() % 127); - return (c < 32) ? 32 : c; - }); - - auto err = nvs_set_str(handle, keys[index], buf); - if (err == ESP_ERR_FLASH_OP_FAIL) { - return err; - } - REQUIRE(err == ESP_OK); - written[index] = true; - break; - } - - default: - assert(0); - } - return ESP_OK; - }; - - - for (size_t i = 0; i < count; ++i) { - size_t index = gen() % nKeys; - switch (gen() % 3) { - case 0: // read, 1/3 - if (randomRead(index) == ESP_ERR_FLASH_OP_FAIL) { - return ESP_ERR_FLASH_OP_FAIL; - } - break; - - default: // write, 2/3 - if (randomWrite(index) == ESP_ERR_FLASH_OP_FAIL) { - return ESP_ERR_FLASH_OP_FAIL; - } - break; - } +public: + RandomTest() + { + std::fill_n(written, nKeys, false); } - return ESP_OK; -} + + template + esp_err_t doRandomThings(nvs_handle handle, TGen gen, size_t& count) { + + const char* keys[] = {"foo", "bar", "longkey_0123456", "another key", "param1", "param2", "param3", "param4", "param5"}; + const ItemType types[] = {ItemType::I32, ItemType::I32, ItemType::U64, ItemType::U64, ItemType::SZ, ItemType::SZ, ItemType::SZ, ItemType::SZ, ItemType::SZ}; + + void* values[] = {&v1, &v2, &v3, &v4, &v5, &v6, &v7, &v8, &v9}; + + const size_t nKeys = sizeof(keys) / sizeof(keys[0]); + static_assert(nKeys == sizeof(types) / sizeof(types[0]), ""); + static_assert(nKeys == sizeof(values) / sizeof(values[0]), ""); + + + auto generateRandomString = [](char* dst, size_t size) { + size_t len = 0; + }; + + auto randomRead = [&](size_t index) -> esp_err_t { + switch (types[index]) { + case ItemType::I32: + { + int32_t val; + auto err = nvs_get_i32(handle, keys[index], &val); + if (err == ESP_ERR_FLASH_OP_FAIL) { + return err; + } + if (!written[index]) { + REQUIRE(err == ESP_ERR_NVS_NOT_FOUND); + } + else { + REQUIRE(err == ESP_OK); + REQUIRE(val == *reinterpret_cast(values[index])); + } + break; + } + + case ItemType::U64: + { + uint64_t val; + auto err = nvs_get_u64(handle, keys[index], &val); + if (err == ESP_ERR_FLASH_OP_FAIL) { + return err; + } + if (!written[index]) { + REQUIRE(err == ESP_ERR_NVS_NOT_FOUND); + } + else { + REQUIRE(err == ESP_OK); + REQUIRE(val == *reinterpret_cast(values[index])); + } + break; + } + + case ItemType::SZ: + { + char buf[strBufLen]; + size_t len = strBufLen; + auto err = nvs_get_str(handle, keys[index], buf, &len); + if (err == ESP_ERR_FLASH_OP_FAIL) { + return err; + } + if (!written[index]) { + REQUIRE(err == ESP_ERR_NVS_NOT_FOUND); + } + else { + REQUIRE(err == ESP_OK); + REQUIRE(strncmp(buf, reinterpret_cast(values[index]), strBufLen) == 0); + } + break; + } + + default: + assert(0); + } + return ESP_OK; + }; + + auto randomWrite = [&](size_t index) -> esp_err_t { + switch (types[index]) { + case ItemType::I32: + { + int32_t val = static_cast(gen()); + + auto err = nvs_set_i32(handle, keys[index], val); + if (err == ESP_ERR_FLASH_OP_FAIL) { + return err; + } + if (err == ESP_ERR_NVS_REMOVE_FAILED) { + written[index] = true; + *reinterpret_cast(values[index]) = val; + return ESP_ERR_FLASH_OP_FAIL; + } + REQUIRE(err == ESP_OK); + written[index] = true; + *reinterpret_cast(values[index]) = val; + break; + } + + case ItemType::U64: + { + uint64_t val = static_cast(gen()); + + auto err = nvs_set_u64(handle, keys[index], val); + if (err == ESP_ERR_FLASH_OP_FAIL) { + return err; + } + if (err == ESP_ERR_NVS_REMOVE_FAILED) { + written[index] = true; + *reinterpret_cast(values[index]) = val; + return ESP_ERR_FLASH_OP_FAIL; + } + REQUIRE(err == ESP_OK); + written[index] = true; + *reinterpret_cast(values[index]) = val; + break; + } + + case ItemType::SZ: + { + char buf[strBufLen]; + size_t len = strBufLen; + + size_t strLen = gen() % (strBufLen - 1); + std::generate_n(buf, strLen, [&]() -> char { + const char c = static_cast(gen() % 127); + return (c < 32) ? 32 : c; + }); + buf[strLen] = 0; + + auto err = nvs_set_str(handle, keys[index], buf); + if (err == ESP_ERR_FLASH_OP_FAIL) { + return err; + } + if (err == ESP_ERR_NVS_REMOVE_FAILED) { + written[index] = true; + strlcpy(reinterpret_cast(values[index]), buf, strBufLen); + return ESP_ERR_FLASH_OP_FAIL; + } + REQUIRE(err == ESP_OK); + written[index] = true; + strlcpy(reinterpret_cast(values[index]), buf, strBufLen); + break; + } + + default: + assert(0); + } + return ESP_OK; + }; + + + for (; count != 0; --count) { + size_t index = gen() % nKeys; + switch (gen() % 3) { + case 0: // read, 1/3 + if (randomRead(index) == ESP_ERR_FLASH_OP_FAIL) { + return ESP_ERR_FLASH_OP_FAIL; + } + break; + + default: // write, 2/3 + if (randomWrite(index) == ESP_ERR_FLASH_OP_FAIL) { + return ESP_ERR_FLASH_OP_FAIL; + } + break; + } + } + return ESP_OK; + } +}; + TEST_CASE("monkey test", "[nvs][monkey]") { @@ -693,6 +723,7 @@ TEST_CASE("monkey test", "[nvs][monkey]") SpiFlashEmulator emu(10); emu.randomize(seed); + emu.clearStats(); const uint32_t NVS_FLASH_SECTOR = 6; const uint32_t NVS_FLASH_SECTOR_COUNT_MIN = 3; @@ -702,9 +733,66 @@ TEST_CASE("monkey test", "[nvs][monkey]") nvs_handle handle; TEST_ESP_OK(nvs_open("namespace1", NVS_READWRITE, &handle)); + RandomTest test; + size_t count = 1000; + CHECK(test.doRandomThings(handle, gen, count) == ESP_OK); + + s_perf << "Monkey test: nErase=" << emu.getEraseOps() << " nWrite=" << emu.getWriteOps() << std::endl; +} - CHECK(doRandomThings(handle, gen, 10000) == ESP_OK); +TEST_CASE("test recovery from sudden poweroff", "[nvs][recovery]") +{ + std::random_device rd; + std::mt19937 gen(rd()); + uint32_t seed = 3; + gen.seed(seed); + const size_t iter_count = 2000; + + SpiFlashEmulator emu(10); + + const uint32_t NVS_FLASH_SECTOR = 6; + const uint32_t NVS_FLASH_SECTOR_COUNT_MIN = 3; + emu.setBounds(NVS_FLASH_SECTOR, NVS_FLASH_SECTOR + NVS_FLASH_SECTOR_COUNT_MIN); + + size_t totalOps = 0; + int lastPercent = -1; + for (uint32_t errDelay = 4; ; ++errDelay) { + INFO(errDelay); + emu.randomize(seed); + emu.clearStats(); + emu.failAfter(errDelay); + RandomTest test; + + if (totalOps != 0) { + int percent = errDelay * 100 / totalOps; + if (percent != lastPercent) { + printf("%d/%d (%d%%)\r\n", errDelay, static_cast(totalOps), percent); + lastPercent = percent; + } + } + + TEST_ESP_OK(nvs_flash_init(NVS_FLASH_SECTOR, NVS_FLASH_SECTOR_COUNT_MIN)); + nvs_handle handle; + TEST_ESP_OK(nvs_open("namespace1", NVS_READWRITE, &handle)); + + size_t count = iter_count; + if(test.doRandomThings(handle, gen, count) != ESP_ERR_FLASH_OP_FAIL) { + nvs_close(handle); + break; + } + nvs_close(handle); + + TEST_ESP_OK(nvs_flash_init(NVS_FLASH_SECTOR, NVS_FLASH_SECTOR_COUNT_MIN)); + TEST_ESP_OK(nvs_open("namespace1", NVS_READWRITE, &handle)); + auto res = test.doRandomThings(handle, gen, count); + if (res != ESP_OK) { + nvs_dump(); + CHECK(0); + } + nvs_close(handle); + totalOps = emu.getEraseOps() + emu.getWriteOps(); + } } TEST_CASE("dump all performance data", "[nvs]") From a65e0194511a30024aa7cb9dc65d7a4cfe546fc1 Mon Sep 17 00:00:00 2001 From: Ivan Grokhotkov Date: Mon, 22 Aug 2016 14:16:21 +0800 Subject: [PATCH 6/9] components/nvs: fix host build with GCC 4.9, add coverage report generation --- components/nvs_flash/.gitignore | 6 +++++ components/nvs_flash/test/Makefile | 24 +++++++++++++++---- .../nvs_flash/test/spi_flash_emulation.h | 6 ++--- components/nvs_flash/test/test_nvs.cpp | 5 ---- 4 files changed, 29 insertions(+), 12 deletions(-) diff --git a/components/nvs_flash/.gitignore b/components/nvs_flash/.gitignore index ba6fca0386..f07085166b 100644 --- a/components/nvs_flash/.gitignore +++ b/components/nvs_flash/.gitignore @@ -1 +1,7 @@ test/test_nvs +test/coverage +test/coverage.info +*.gcno +*.gcda +*.gcov +*.o diff --git a/components/nvs_flash/test/Makefile b/components/nvs_flash/test/Makefile index ded1235d70..8fa7b3efe3 100644 --- a/components/nvs_flash/test/Makefile +++ b/components/nvs_flash/test/Makefile @@ -1,5 +1,5 @@ TEST_PROGRAM=test_nvs -all: test +all: $(TEST_PROGRAM) SOURCE_FILES = \ $(addprefix ../src/, \ @@ -14,15 +14,18 @@ SOURCE_FILES = \ test_spi_flash_emulation.cpp \ test_intrusive_list.cpp \ test_nvs.cpp \ - crc.cpp \ + crc.cpp \ main.cpp -CPPFLAGS += -I../include -I../src -I./ -I../../esp32/include -I ../../spi_flash/include +CPPFLAGS += -I../include -I../src -I./ -I../../esp32/include -I ../../spi_flash/include -fprofile-arcs -ftest-coverage +CFLAGS += -fprofile-arcs -ftest-coverage CXXFLAGS += -std=c++11 -Wall -Werror -LDFLAGS += -lstdc++ +LDFLAGS += -lstdc++ -Wall -fprofile-arcs -ftest-coverage OBJ_FILES = $(SOURCE_FILES:.cpp=.o) +COVERAGE_FILES = $(OBJ_FILES:.o=.gc*) + $(OBJ_FILES): %.o: %.cpp $(TEST_PROGRAM): $(OBJ_FILES) @@ -34,7 +37,20 @@ $(OUTPUT_DIR): test: $(TEST_PROGRAM) ./$(TEST_PROGRAM) +$(COVERAGE_FILES): $(TEST_PROGRAM) test + +coverage.info: $(COVERAGE_FILES) + find ../src/ -name "*.gcno" -exec gcov -r -pb {} + + lcov --capture --directory ../src --no-external --output-file coverage.info + +coverage_report: coverage.info + genhtml coverage.info --output-directory coverage_report + @echo "Coverage report is in coverage_report/index.html" + clean: rm -f $(OBJ_FILES) $(TEST_PROGRAM) + rm -f $(COVERAGE_FILES) *.gcov + rm -rf coverage_report/ + rm -f coverage.info .PHONY: clean all test diff --git a/components/nvs_flash/test/spi_flash_emulation.h b/components/nvs_flash/test/spi_flash_emulation.h index ca1f9e27f5..d5a242b240 100644 --- a/components/nvs_flash/test/spi_flash_emulation.h +++ b/components/nvs_flash/test/spi_flash_emulation.h @@ -74,7 +74,7 @@ public: return false; } - if (mFailCountdown != SIZE_T_MAX && mFailCountdown-- == 0) { + if (mFailCountdown != SIZE_MAX && mFailCountdown-- == 0) { return false; } @@ -108,7 +108,7 @@ public: return false; } - if (mFailCountdown != SIZE_T_MAX && mFailCountdown-- == 0) { + if (mFailCountdown != SIZE_MAX && mFailCountdown-- == 0) { return false; } @@ -203,7 +203,7 @@ protected: size_t mLowerSectorBound = 0; size_t mUpperSectorBound = 0; - size_t mFailCountdown = SIZE_T_MAX; + size_t mFailCountdown = SIZE_MAX; }; diff --git a/components/nvs_flash/test/test_nvs.cpp b/components/nvs_flash/test/test_nvs.cpp index 6e8f4124fc..beaa79e816 100644 --- a/components/nvs_flash/test/test_nvs.cpp +++ b/components/nvs_flash/test/test_nvs.cpp @@ -554,11 +554,6 @@ public: static_assert(nKeys == sizeof(types) / sizeof(types[0]), ""); static_assert(nKeys == sizeof(values) / sizeof(values[0]), ""); - - auto generateRandomString = [](char* dst, size_t size) { - size_t len = 0; - }; - auto randomRead = [&](size_t index) -> esp_err_t { switch (types[index]) { case ItemType::I32: From b783e2ffaba2b3b620ef00b1acb12f08ce96cdeb Mon Sep 17 00:00:00 2001 From: Ivan Grokhotkov Date: Tue, 23 Aug 2016 12:07:23 +0800 Subject: [PATCH 7/9] components/nvs: fix typos in readme --- components/nvs_flash/README.rst | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/components/nvs_flash/README.rst b/components/nvs_flash/README.rst index ccaadee77d..ee8ee0b262 100644 --- a/components/nvs_flash/README.rst +++ b/components/nvs_flash/README.rst @@ -61,7 +61,7 @@ Internals Log of key-value pairs ~~~~~~~~~~~~~~~~~~~~~~ -NVS stores key-value pairs sequentially, with new key-value pairs being added at the end. When a value of any given key has to be updated, old key-value pair is marked as erased and new key-value pair is added at the end of the log. +NVS stores key-value pairs sequentially, with new key-value pairs being added at the end. When a value of any given key has to be updated, new key-value pair is added at the end of the log and old key-value pair is marked as erased. Pages and entries ~~~~~~~~~~~~~~~~~ @@ -69,10 +69,10 @@ Pages and entries NVS library uses two main entities in its operation: pages and entries. Page is a logical structure which stores a portion of the overall log. Logical page corresponds to one physical sector of flash memory. Pages which are in use have a *sequence number* associated with them. Sequence numbers impose an ordering on pages. Higher sequence numbers correspond to pages which were created later. Each page can be in one of the following states: Empty/uninitialized - Flash storage for the page is empty (all bytes are ``0xff``). Page isn't used to store any data at this point and doesn’t have + Flash storage for the page is empty (all bytes are ``0xff``). Page isn't used to store any data at this point and doesn’t have a sequence number. Active - Flash storage is initialized, page header has been written to flash, page has a valid sequence number. Page has some empty entries and data can be written there. Normally only one page can be in this state. + Flash storage is initialized, page header has been written to flash, page has a valid sequence number. Page has some empty entries and data can be written there. At most one page can be in this state at any given moment. Full Flash storage is in a consistent state and is filled with key-value pairs. @@ -213,7 +213,7 @@ As mentioned above, each key-value pair belongs to one of the namespaces. Namesp +-------------------------------------------+ | NS=0 Type=uint8_t Key="pwm" Value=2 | Entry describing namespace "pwm" +-------------------------------------------+ - | NS=0 Type=uint16_t Key="channel" Value=20 | Key "channel" in namespace "pwm" + | NS=2 Type=uint16_t Key="channel" Value=20 | Key "channel" in namespace "pwm" +-------------------------------------------+ From 3df4130eb7554027282f2e202a0e76ed95db301e Mon Sep 17 00:00:00 2001 From: Ivan Grokhotkov Date: Tue, 23 Aug 2016 12:54:09 +0800 Subject: [PATCH 8/9] components/nvs: run small number of tests as part of CI builds --- .gitlab-ci.yml | 10 ++++++++-- components/nvs_flash/test/Makefile | 5 ++++- components/nvs_flash/test/test_nvs.cpp | 2 +- 3 files changed, 13 insertions(+), 4 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index c7ef559ada..3f23ccc089 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -1,9 +1,8 @@ stages: - build -# - test + - test - deploy - build_template_app: stage: build image: espressif/esp32-ci-env @@ -18,6 +17,13 @@ build_template_app: - cd esp-idf-template - make all +test_nvs_on_host: + stage: test + image: espressif/esp32-ci-env + script: + - cd components/nvs_flash/test + - make test + push_master_to_github: stage: deploy only: diff --git a/components/nvs_flash/test/Makefile b/components/nvs_flash/test/Makefile index 8fa7b3efe3..0b4de9c6b2 100644 --- a/components/nvs_flash/test/Makefile +++ b/components/nvs_flash/test/Makefile @@ -37,7 +37,10 @@ $(OUTPUT_DIR): test: $(TEST_PROGRAM) ./$(TEST_PROGRAM) -$(COVERAGE_FILES): $(TEST_PROGRAM) test +long-test: $(TEST_PROGRAM) + ./$(TEST_PROGRAM) [list],[enumtable],[spi_flash_emu],[nvs],[long] + +$(COVERAGE_FILES): $(TEST_PROGRAM) long-test coverage.info: $(COVERAGE_FILES) find ../src/ -name "*.gcno" -exec gcov -r -pb {} + diff --git a/components/nvs_flash/test/test_nvs.cpp b/components/nvs_flash/test/test_nvs.cpp index beaa79e816..c137d9c6cc 100644 --- a/components/nvs_flash/test/test_nvs.cpp +++ b/components/nvs_flash/test/test_nvs.cpp @@ -735,7 +735,7 @@ TEST_CASE("monkey test", "[nvs][monkey]") s_perf << "Monkey test: nErase=" << emu.getEraseOps() << " nWrite=" << emu.getWriteOps() << std::endl; } -TEST_CASE("test recovery from sudden poweroff", "[nvs][recovery]") +TEST_CASE("test recovery from sudden poweroff", "[.][long][nvs][recovery][monkey]") { std::random_device rd; std::mt19937 gen(rd()); From 9ef827ae20c22f570ba2486d543c1c83f57267da Mon Sep 17 00:00:00 2001 From: Ivan Grokhotkov Date: Tue, 23 Aug 2016 15:14:13 +0800 Subject: [PATCH 9/9] components/nvs: strlcpy is not available on Linux, replace with strncpy and terminate strings explicitly --- components/nvs_flash/src/nvs_page.cpp | 6 +++--- components/nvs_flash/src/nvs_storage.cpp | 5 +++-- components/nvs_flash/test/Makefile | 2 +- components/nvs_flash/test/test_intrusive_list.cpp | 3 ++- components/nvs_flash/test/test_nvs.cpp | 6 +++--- 5 files changed, 12 insertions(+), 10 deletions(-) diff --git a/components/nvs_flash/src/nvs_page.cpp b/components/nvs_flash/src/nvs_page.cpp index 918908dd6f..cddb69393b 100644 --- a/components/nvs_flash/src/nvs_page.cpp +++ b/components/nvs_flash/src/nvs_page.cpp @@ -18,7 +18,7 @@ #include "crc.h" #endif #include - +#include namespace nvs { @@ -156,8 +156,8 @@ esp_err_t Page::writeItem(uint8_t nsIndex, ItemType datatype, const char* key, c std::fill_n(reinterpret_cast(item.key), sizeof(item.key) / 4, 0xffffffff); std::fill_n(reinterpret_cast(item.data), sizeof(item.data) / 4, 0xffffffff); - strlcpy(item.key, key, Item::MAX_KEY_LENGTH + 1); - + strncpy(item.key, key, sizeof(item.key) - 1); + item.key[sizeof(item.key) - 1] = 0; if (datatype != ItemType::SZ && datatype != ItemType::BLOB) { memcpy(item.data, data, dataSize); diff --git a/components/nvs_flash/src/nvs_storage.cpp b/components/nvs_flash/src/nvs_storage.cpp index 5cbc52bee3..9be6580a1d 100644 --- a/components/nvs_flash/src/nvs_storage.cpp +++ b/components/nvs_flash/src/nvs_storage.cpp @@ -161,7 +161,8 @@ esp_err_t Storage::createOrOpenNamespace(const char* nsName, bool canCreate, uin NamespaceEntry* entry = new NamespaceEntry; entry->mIndex = ns; - strlcpy(entry->mName, nsName, sizeof(entry->mName)); + strncpy(entry->mName, nsName, sizeof(entry->mName) - 1); + entry->mName[sizeof(entry->mName) - 1] = 0; mNamespaces.push_back(entry); } else { @@ -250,4 +251,4 @@ void Storage::debugCheck() } #endif //ESP_PLATFORM -} \ No newline at end of file +} diff --git a/components/nvs_flash/test/Makefile b/components/nvs_flash/test/Makefile index 0b4de9c6b2..d14dbaa3c9 100644 --- a/components/nvs_flash/test/Makefile +++ b/components/nvs_flash/test/Makefile @@ -29,7 +29,7 @@ COVERAGE_FILES = $(OBJ_FILES:.o=.gc*) $(OBJ_FILES): %.o: %.cpp $(TEST_PROGRAM): $(OBJ_FILES) - gcc $(LDFLAGS) -o $(TEST_PROGRAM) $(OBJ_FILES) + g++ $(LDFLAGS) -o $(TEST_PROGRAM) $(OBJ_FILES) $(OUTPUT_DIR): mkdir -p $(OUTPUT_DIR) diff --git a/components/nvs_flash/test/test_intrusive_list.cpp b/components/nvs_flash/test/test_intrusive_list.cpp index 8fc7aa95de..979438a0bc 100644 --- a/components/nvs_flash/test/test_intrusive_list.cpp +++ b/components/nvs_flash/test/test_intrusive_list.cpp @@ -19,7 +19,8 @@ struct TestNode : public intrusive_list_node { TestNode(const char* name_ = "", int num_ = 0) : num(num_) { - strlcpy(name, name_, sizeof(name)); + strncpy(name, name_, sizeof(name) - 1); + name[sizeof(name) - 1] = 0; } char name[32]; int num; diff --git a/components/nvs_flash/test/test_nvs.cpp b/components/nvs_flash/test/test_nvs.cpp index c137d9c6cc..325cdcf45b 100644 --- a/components/nvs_flash/test/test_nvs.cpp +++ b/components/nvs_flash/test/test_nvs.cpp @@ -62,7 +62,7 @@ TEST_CASE("crc32 behaves as expected", "[nvs]") CHECK(crc32_1 != item2.calculateCrc32()); item2 = item1; - strlcpy(item2.key, "foo", Item::MAX_KEY_LENGTH); + strncpy(item2.key, "foo", Item::MAX_KEY_LENGTH); CHECK(crc32_1 != item2.calculateCrc32()); } @@ -672,12 +672,12 @@ public: } if (err == ESP_ERR_NVS_REMOVE_FAILED) { written[index] = true; - strlcpy(reinterpret_cast(values[index]), buf, strBufLen); + strncpy(reinterpret_cast(values[index]), buf, strBufLen); return ESP_ERR_FLASH_OP_FAIL; } REQUIRE(err == ESP_OK); written[index] = true; - strlcpy(reinterpret_cast(values[index]), buf, strBufLen); + strncpy(reinterpret_cast(values[index]), buf, strBufLen); break; }