nvs: fix Page::findItem and Storage::findItem regression

When read caching was added, Page::findItem started modifying itemIndex reference argument even if item wasn't found.
Incidentally, Storage::findItem reused itemIndex when starting search at next page.
So,
- if the first page had a cached index (findItem was called for that page), and it pointed to a non-zero index,
- first page has a few empty items at the end (but is marked full),
- next search looked up the item on the second page,
- index of the item on the second page was less than the cached index on the first page,
then the search would fail because cached starting index was reused.

This change fixes both sides of the problem:
- Page::findItem shouldn't modify itemIndex argument if item is not found
- Storage::findItem should not reuse itemIndex between pages

Two tests have been added.
This commit is contained in:
Ivan Grokhotkov 2016-11-03 20:18:30 +08:00
parent abecab7525
commit e314f42b0c
3 changed files with 61 additions and 6 deletions

View file

@ -656,19 +656,20 @@ 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) {
size_t findBeginIndex = itemIndex;
if (findBeginIndex >= ENTRY_COUNT) {
return ESP_ERR_NVS_NOT_FOUND;
}
CachedFindInfo findInfo(nsIndex, datatype, key);
if (mFindInfo == findInfo) {
itemIndex = mFindInfo.itemIndex();
findBeginIndex = mFindInfo.itemIndex();
}
size_t start = mFirstUsedEntry;
if (itemIndex > mFirstUsedEntry && itemIndex < ENTRY_COUNT) {
start = itemIndex;
if (findBeginIndex > mFirstUsedEntry && findBeginIndex < ENTRY_COUNT) {
start = findBeginIndex;
}
size_t end = mNextFreeEntry;

View file

@ -71,8 +71,8 @@ esp_err_t Storage::init(uint32_t baseSector, uint32_t sectorCount)
esp_err_t Storage::findItem(uint8_t nsIndex, ItemType datatype, const char* key, Page* &page, Item& item)
{
size_t itemIndex = 0;
for (auto it = std::begin(mPageManager); it != std::end(mPageManager); ++it) {
size_t itemIndex = 0;
auto err = it->findItem(nsIndex, datatype, key, itemIndex, item);
if (err == ESP_OK) {
page = it;

View file

@ -300,6 +300,27 @@ TEST_CASE("storage doesn't add duplicates within multiple pages", "[nvs]")
CHECK(page.findItem(1, itemTypeOf<int>(), "bar") == ESP_OK);
}
TEST_CASE("storage can find items on second page if first is not fully written and has cached search data", "[nvs]")
{
SpiFlashEmulator emu(3);
Storage storage;
CHECK(storage.init(0, 3) == ESP_OK);
int bar = 0;
uint8_t bigdata[100 * 32] = {0};
// write one big chunk of data
ESP_ERROR_CHECK(storage.writeItem(0, ItemType::BLOB, "first", bigdata, sizeof(bigdata)));
// write second one; it will not fit into the first page
ESP_ERROR_CHECK(storage.writeItem(0, ItemType::BLOB, "second", bigdata, sizeof(bigdata)));
size_t size;
ESP_ERROR_CHECK(storage.getItemDataSize(0, ItemType::BLOB, "first", size));
CHECK(size == sizeof(bigdata));
ESP_ERROR_CHECK(storage.getItemDataSize(0, ItemType::BLOB, "second", size));
CHECK(size == sizeof(bigdata));
}
TEST_CASE("can write and read variable length data lots of times", "[nvs]")
{
SpiFlashEmulator emu(8);
@ -1055,6 +1076,39 @@ TEST_CASE("crc error in variable length item is handled", "[nvs]")
}
TEST_CASE("read/write failure (TW8406)", "[nvs]")
{
SpiFlashEmulator emu(3);
nvs_flash_init_custom(0, 3);
for (int attempts = 0; attempts < 3; ++attempts) {
int i = 0;
nvs_handle light_handle = 0;
char key[15] = {0};
char data[76] = {12, 13, 14, 15, 16};
uint8_t number = 20;
size_t data_len = sizeof(data);
ESP_ERROR_CHECK(nvs_open("LIGHT", NVS_READWRITE, &light_handle));
ESP_ERROR_CHECK(nvs_set_u8(light_handle, "RecordNum", number));
for (i = 0; i < number; ++i) {
sprintf(key, "light%d", i);
ESP_ERROR_CHECK(nvs_set_blob(light_handle, key, data, sizeof(data)));
}
nvs_commit(light_handle);
uint8_t get_number = 0;
ESP_ERROR_CHECK(nvs_get_u8(light_handle, "RecordNum", &get_number));
REQUIRE(number == get_number);
for (i = 0; i < number; ++i) {
char data[76] = {0};
sprintf(key, "light%d", i);
ESP_ERROR_CHECK(nvs_get_blob(light_handle, key, data, &data_len));
}
nvs_close(light_handle);
}
}
TEST_CASE("dump all performance data", "[nvs]")
{
std::cout << "====================" << std::endl << "Dumping benchmarks" << std::endl;