Merge branch 'bugfix/nvs_failed_crc_assert' into 'master'

nvs: don’t expect items with bad CRC to be in cache

See merge request idf/esp-idf!2239
This commit is contained in:
Ivan Grokhotkov 2018-04-18 11:01:21 +08:00
commit 17ab60d642
4 changed files with 38 additions and 13 deletions

View file

@ -60,7 +60,7 @@ void HashList::insert(const Item& item, size_t index)
newBlock->mCount++;
}
void HashList::erase(size_t index)
void HashList::erase(size_t index, bool itemShouldExist)
{
for (auto it = mBlockList.begin(); it != mBlockList.end();) {
bool haveEntries = false;
@ -82,8 +82,10 @@ void HashList::erase(size_t index)
++it;
}
}
if (itemShouldExist) {
assert(false && "item should have been present in cache");
}
}
size_t HashList::find(size_t start, const Item& item)
{

View file

@ -29,7 +29,7 @@ public:
~HashList();
void insert(const Item& item, size_t index);
void erase(const size_t index);
void erase(const size_t index, bool itemShouldExist=true);
size_t find(size_t start, const Item& item);
void clear();

View file

@ -314,7 +314,6 @@ esp_err_t Page::eraseEntryAndSpan(size_t index)
{
auto state = mEntryTable.get(index);
assert(state == EntryState::WRITTEN || state == EntryState::EMPTY);
mHashList.erase(index);
size_t span = 1;
if (state == EntryState::WRITTEN) {
@ -324,6 +323,7 @@ esp_err_t Page::eraseEntryAndSpan(size_t index)
return rc;
}
if (item.calculateCrc32() != item.crc32) {
mHashList.erase(index, false);
rc = alterEntryState(index, EntryState::ERASED);
--mUsedEntryCount;
++mErasedEntryCount;
@ -331,6 +331,7 @@ esp_err_t Page::eraseEntryAndSpan(size_t index)
return rc;
}
} else {
mHashList.erase(index);
span = item.span;
for (ptrdiff_t i = index + span - 1; i >= static_cast<ptrdiff_t>(index); --i) {
if (mEntryTable.get(i) == EntryState::WRITTEN) {
@ -511,11 +512,6 @@ esp_err_t Page::mLoadEntryTable()
return err;
}
mHashList.insert(item, i);
// search for potential duplicate item
size_t duplicateIndex = mHashList.find(0, item);
if (item.crc32 != item.calculateCrc32()) {
err = eraseEntryAndSpan(i);
if (err != ESP_OK) {
@ -525,6 +521,10 @@ esp_err_t Page::mLoadEntryTable()
continue;
}
mHashList.insert(item, i);
// search for potential duplicate item
size_t duplicateIndex = mHashList.find(0, item);
if (item.datatype == ItemType::BLOB || item.datatype == ItemType::SZ) {
span = item.span;
@ -576,8 +576,6 @@ esp_err_t Page::mLoadEntryTable()
return err;
}
mHashList.insert(item, i);
if (item.crc32 != item.calculateCrc32()) {
err = eraseEntryAndSpan(i);
if (err != ESP_OK) {
@ -589,6 +587,8 @@ esp_err_t Page::mLoadEntryTable()
assert(item.span > 0);
mHashList.insert(item, i);
size_t span = item.span;
if (item.datatype == ItemType::BLOB || item.datatype == ItemType::SZ) {
@ -736,7 +736,11 @@ esp_err_t Page::findItem(uint8_t nsIndex, ItemType datatype, const char* key, si
auto crc32 = item.calculateCrc32();
if (item.crc32 != crc32) {
eraseEntryAndSpan(i);
rc = eraseEntryAndSpan(i);
if (rc != ESP_OK) {
mState = PageState::INVALID;
return rc;
}
continue;
}

View file

@ -259,6 +259,25 @@ TEST_CASE("Page validates blob size", "[nvs]")
TEST_ESP_OK(page.writeItem(1, ItemType::BLOB, "2", buf, Page::BLOB_MAX_SIZE));
}
TEST_CASE("Page handles invalid CRC of variable length items", "[nvs][cur]")
{
SpiFlashEmulator emu(4);
{
Page page;
TEST_ESP_OK(page.load(0));
char buf[128] = {0};
TEST_ESP_OK(page.writeItem(1, ItemType::BLOB, "1", buf, sizeof(buf)));
}
// corrupt header of the item (64 is the offset of the first item in page)
uint32_t overwrite_buf = 0;
emu.write(64, &overwrite_buf, 4);
// load page again
{
Page page;
TEST_ESP_OK(page.load(0));
}
}
TEST_CASE("can init PageManager in empty flash", "[nvs]")
{
SpiFlashEmulator emu(4);