Crash_Override/OVMS3-idf
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

Merge branch 'feature/nvs_check_item_modified' into 'master'

Browse source 
nvs: Check if an item is modified before writing out an identical copy

See merge request idf/esp-idf!4934
This commit is contained in:
Ivan Grokhotkov 2019-05-21 10:54:21 +08:00
parent ef05ccf666 c3fa2497b9
commit 0ccb7541f5
6 changed files with 176 additions and 5 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
components/nvs_flash/include/nvs.h
View file

@ -54,6 +54,7 @@ typedef uint32_t nvs_handle;
#define ESP_ERR_NVS_KEYS_NOT_INITIALIZED (ESP_ERR_NVS_BASE + 0x16) /*!< NVS key partition is uninitialized */
#define ESP_ERR_NVS_CORRUPT_KEY_PART (ESP_ERR_NVS_BASE + 0x17) /*!< NVS key partition is corrupt */
#define ESP_ERR_NVS_CONTENT_DIFFERS (ESP_ERR_NVS_BASE + 0x18) /*!< Internal error; never returned by nvs API functions. NVS key is different in comparison */
#define NVS_DEFAULT_PART_NAME "nvs" /*!< Default partition name of the NVS partition in the partition table */
/**

52
components/nvs_flash/src/nvs_page.cpp
View file

@ -308,6 +308,58 @@ esp_err_t Page::readItem(uint8_t nsIndex, ItemType datatype, const char* key, vo
return ESP_OK;
}
esp_err_t Page::cmpItem(uint8_t nsIndex, ItemType datatype, const char* key, const void* data, size_t dataSize, uint8_t chunkIdx, VerOffset chunkStart)
{
size_t index = 0;
Item item;
if (mState == PageState::INVALID) {
return ESP_ERR_NVS_INVALID_STATE;
}
esp_err_t rc = findItem(nsIndex, datatype, key, index, item, chunkIdx, chunkStart);
if (rc != ESP_OK) {
return rc;
}
if (!isVariableLengthType(datatype)) {
if (dataSize != getAlignmentForType(datatype)) {
return ESP_ERR_NVS_TYPE_MISMATCH;
}
if (memcmp(data, item.data, dataSize)) {
return ESP_ERR_NVS_CONTENT_DIFFERS;
}
return ESP_OK;
}
if (dataSize < static_cast<size_t>(item.varLength.dataSize)) {
return ESP_ERR_NVS_INVALID_LENGTH;
}
const uint8_t* dst = reinterpret_cast<const uint8_t*>(data);
size_t left = item.varLength.dataSize;
for (size_t i = index + 1; i < index + item.span; ++i) {
Item ditem;
rc = readEntry(i, ditem);
if (rc != ESP_OK) {
return rc;
}
size_t willCopy = ENTRY_SIZE;
willCopy = (left < willCopy)?left:willCopy;
if (memcmp(dst, ditem.rawData, willCopy)) {
return ESP_ERR_NVS_CONTENT_DIFFERS;
}
left -= willCopy;
dst += willCopy;
}
if (Item::calculateCrc32(reinterpret_cast<const uint8_t*>(data), item.varLength.dataSize) != item.varLength.dataCrc32) {
return ESP_ERR_NVS_NOT_FOUND;
}
return ESP_OK;
}
esp_err_t Page::eraseItem(uint8_t nsIndex, ItemType datatype, const char* key, uint8_t chunkIdx, VerOffset chunkStart)
{
size_t index = 0;

8
components/nvs_flash/src/nvs_page.hpp
View file

@ -94,6 +94,8 @@ public:
esp_err_t readItem(uint8_t nsIndex, ItemType datatype, const char* key, void* data, size_t dataSize, uint8_t chunkIdx = CHUNK_ANY, VerOffset chunkStart = VerOffset::VER_ANY);
esp_err_t cmpItem(uint8_t nsIndex, ItemType datatype, const char* key, const void* data, size_t dataSize, uint8_t chunkIdx = CHUNK_ANY, VerOffset chunkStart = VerOffset::VER_ANY);
esp_err_t eraseItem(uint8_t nsIndex, ItemType datatype, const char* key, uint8_t chunkIdx = CHUNK_ANY, VerOffset chunkStart = VerOffset::VER_ANY);
esp_err_t findItem(uint8_t nsIndex, ItemType datatype, const char* key, uint8_t chunkIdx = CHUNK_ANY, VerOffset chunkStart = VerOffset::VER_ANY);
@ -112,6 +114,12 @@ public:
return readItem(nsIndex, itemTypeOf(value), key, &value, sizeof(value));
}
template<typename T>
esp_err_t cmpItem(uint8_t nsIndex, const char* key, const T& value)
{
return cmpItem(nsIndex, itemTypeOf(value), key, &value, sizeof(value));
}
template<typename T>
esp_err_t eraseItem(uint8_t nsIndex, const char* key)
{

56
components/nvs_flash/src/nvs_storage.cpp
View file

@ -269,6 +269,13 @@ esp_err_t Storage::writeItem(uint8_t nsIndex, ItemType datatype, const char* key
VerOffset prevStart, nextStart;
prevStart = nextStart = VerOffset::VER_0_OFFSET;
if (findPage) {
// Do a sanity 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) {
return ESP_OK;
}
if (findPage->state() == Page::PageState::UNINITIALIZED ||
findPage->state() == Page::PageState::INVALID) {
ESP_ERROR_CHECK(findItem(nsIndex, datatype, key, findPage, item));
@ -311,6 +318,13 @@ esp_err_t Storage::writeItem(uint8_t nsIndex, ItemType datatype, const char* key
}
}
} else {
// Do a sanity 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 (findPage != nullptr &&
findPage->cmpItem(nsIndex, datatype, key, data, dataSize) == ESP_OK) {
return ESP_OK;
}
Page& page = getCurrentPage();
err = page.writeItem(nsIndex, datatype, key, data, dataSize);
@ -443,6 +457,48 @@ esp_err_t Storage::readMultiPageBlob(uint8_t nsIndex, const char* key, void* dat
return err;
}
esp_err_t Storage::cmpMultiPageBlob(uint8_t nsIndex, const char* key, const void* data, size_t dataSize)
{
Item item;
Page* findPage = nullptr;
/* First read the blob index */
auto err = findItem(nsIndex, ItemType::BLOB_IDX, key, findPage, item);
if (err != ESP_OK) {
return err;
}
uint8_t chunkCount = item.blobIndex.chunkCount;
VerOffset chunkStart = item.blobIndex.chunkStart;
size_t readSize = item.blobIndex.dataSize;
size_t offset = 0;
if (dataSize != readSize) {
return ESP_ERR_NVS_CONTENT_DIFFERS;
}
/* Now read corresponding chunks */
for (uint8_t chunkNum = 0; chunkNum < chunkCount; chunkNum++) {
err = findItem(nsIndex, ItemType::BLOB_DATA, key, findPage, item, static_cast<uint8_t> (chunkStart) + chunkNum);
if (err != ESP_OK) {
if (err == ESP_ERR_NVS_NOT_FOUND) {
break;
}
return err;
}
err = findPage->cmpItem(nsIndex, ItemType::BLOB_DATA, key, static_cast<const uint8_t*>(data) + offset, item.varLength.dataSize, static_cast<uint8_t> (chunkStart) + chunkNum);
if (err != ESP_OK) {
return err;
}
assert(static_cast<uint8_t> (chunkStart) + chunkNum == item.chunkIndex);
offset += item.varLength.dataSize;
}
if (err == ESP_OK) {
assert(offset == dataSize);
}
return err;
}
esp_err_t Storage::readItem(uint8_t nsIndex, ItemType datatype, const char* key, void* data, size_t dataSize)
{
if (mState != StorageState::ACTIVE) {

2
components/nvs_flash/src/nvs_storage.hpp
View file

@ -109,6 +109,8 @@ public:
esp_err_t readMultiPageBlob(uint8_t nsIndex, const char* key, void* data, size_t dataSize);
esp_err_t cmpMultiPageBlob(uint8_t nsIndex, const char* key, const void* data, size_t dataSize);
esp_err_t eraseMultiPageBlob(uint8_t nsIndex, const char* key, VerOffset chunkStart = VerOffset::VER_ANY);
void debugDump();

62
components/nvs_flash/test_nvs_host/test_nvs.cpp
View file

@ -376,8 +376,8 @@ TEST_CASE("storage doesn't add duplicates within one page", "[nvs]")
emu.setBounds(4, 8);
CHECK(storage.init(4, 4) == ESP_OK);
int bar = 0;
CHECK(storage.writeItem(1, "bar", bar) == ESP_OK);
CHECK(storage.writeItem(1, "bar", bar) == ESP_OK);
CHECK(storage.writeItem(1, "bar", ++bar) == ESP_OK);
CHECK(storage.writeItem(1, "bar", ++bar) == ESP_OK);
Page page;
page.load(4);
@ -404,11 +404,11 @@ TEST_CASE("storage doesn't add duplicates within multiple pages", "[nvs]")
emu.setBounds(4, 8);
CHECK(storage.init(4, 4) == ESP_OK);
int bar = 0;
CHECK(storage.writeItem(1, "bar", bar) == ESP_OK);
CHECK(storage.writeItem(1, "bar", ++bar) == ESP_OK);
for (size_t i = 0; i < Page::ENTRY_COUNT; ++i) {
CHECK(storage.writeItem(1, "foo", static_cast<int>(bar)) == ESP_OK);
CHECK(storage.writeItem(1, "foo", static_cast<int>(++bar)) == ESP_OK);
}
CHECK(storage.writeItem(1, "bar", bar) == ESP_OK);
CHECK(storage.writeItem(1, "bar", ++bar) == ESP_OK);
Page page;
page.load(4);
@ -750,6 +750,58 @@ TEST_CASE("wifi test", "[nvs]")
}
TEST_CASE("writing the identical content does not write or erase", "[nvs]")
{
SpiFlashEmulator emu(20);
const uint32_t NVS_FLASH_SECTOR = 5;
const uint32_t NVS_FLASH_SECTOR_COUNT_MIN = 10;
emu.setBounds(NVS_FLASH_SECTOR, NVS_FLASH_SECTOR + NVS_FLASH_SECTOR_COUNT_MIN);
TEST_ESP_OK(nvs_flash_init_custom(NVS_DEFAULT_PART_NAME, NVS_FLASH_SECTOR, NVS_FLASH_SECTOR_COUNT_MIN));
nvs_handle misc_handle;
TEST_ESP_OK(nvs_open("test", NVS_READWRITE, &misc_handle));
// Test writing a u8 twice, then changing it
nvs_set_u8(misc_handle, "test_u8", 8);
emu.clearStats();
nvs_set_u8(misc_handle, "test_u8", 8);
CHECK(emu.getWriteOps() == 0);
CHECK(emu.getEraseOps() == 0);
CHECK(emu.getReadOps() != 0);
emu.clearStats();
nvs_set_u8(misc_handle, "test_u8", 9);
CHECK(emu.getWriteOps() != 0);
CHECK(emu.getReadOps() != 0);
// Test writing a string twice, then changing it
static const char *test[2] = {"Hello world.", "Hello world!"};
nvs_set_str(misc_handle, "test_str", test[0]);
emu.clearStats();
nvs_set_str(misc_handle, "test_str", test[0]);
CHECK(emu.getWriteOps() == 0);
CHECK(emu.getEraseOps() == 0);
CHECK(emu.getReadOps() != 0);
emu.clearStats();
nvs_set_str(misc_handle, "test_str", test[1]);
CHECK(emu.getWriteOps() != 0);
CHECK(emu.getReadOps() != 0);
// Test writing a multi-page blob, then changing it
uint8_t blob[Page::CHUNK_MAX_SIZE * 3] = {0};
memset(blob, 1, sizeof(blob));
nvs_set_blob(misc_handle, "test_blob", blob, sizeof(blob));
emu.clearStats();
nvs_set_blob(misc_handle, "test_blob", blob, sizeof(blob));
CHECK(emu.getWriteOps() == 0);
CHECK(emu.getEraseOps() == 0);
CHECK(emu.getReadOps() != 0);
blob[sizeof(blob) - 1]++;
emu.clearStats();
nvs_set_blob(misc_handle, "test_blob", blob, sizeof(blob));
CHECK(emu.getWriteOps() != 0);
CHECK(emu.getReadOps() != 0);
}
TEST_CASE("can init storage from flash with random contents", "[nvs]")
{