diff --git a/components/nvs_flash/.build-test-rules.yml b/components/nvs_flash/.build-test-rules.yml new file mode 100644 index 0000000000..93b76378a9 --- /dev/null +++ b/components/nvs_flash/.build-test-rules.yml @@ -0,0 +1,3 @@ +components/nvs_flash/host_test: + enable: + - if: IDF_TARGET == "linux" diff --git a/components/nvs_flash/Kconfig b/components/nvs_flash/Kconfig index 38d0a311f3..3689bfa1bf 100644 --- a/components/nvs_flash/Kconfig +++ b/components/nvs_flash/Kconfig @@ -25,4 +25,13 @@ menu "NVS" default n help This option switches error checking type between assertions (y) or return codes (n). + + config NVS_LEGACY_DUP_KEYS_COMPATIBILITY + bool "Enable legacy nvs_set function behavior when same key is reused with different data types" + default n + help + Enabling this will switch the nvs_set family of functions to the legacy mode. When called with same + key and different data type, existing value stored in NVS remains active and as a side effect, the + new value is also stored into NVS, although not accessible using respective nvs_get function. Use only + if your application relies on this NVS API behaviour. endmenu 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 f7f9c6f7f5..44a9f44a9c 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 @@ -3255,6 +3255,71 @@ TEST_CASE("check and read data from partition generated via manufacturing utilit } } +TEST_CASE("nvs multiple write with same key but different types", "[nvs][xxx]") +{ + 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); + + int32_t v32; + int8_t v8; + + TEST_ESP_OK(nvs_set_i32(handle_1, "foo", (int32_t)12345678)); + TEST_ESP_OK(nvs_set_i8(handle_1, "foo", (int8_t)12)); + TEST_ESP_OK(nvs_set_i8(handle_1, "foo", (int8_t)34)); + +#ifdef CONFIG_NVS_LEGACY_DUP_KEYS_COMPATIBILITY + // Legacy behavior + // First use of key hooks data type until removed by nvs_erase_key. Alternative re-use of same key with different + // data type is written to the storage as hidden active value. It is returned by nvs_get function after nvs_erase_key is called. + // Mixing more than 2 data types brings undefined behavior. It is not tested here. + + TEST_ESP_ERR(nvs_get_i8(handle_1, "foo", &v8), ESP_ERR_NVS_NOT_FOUND); + TEST_ESP_OK(nvs_get_i32(handle_1, "foo", &v32)); + TEST_ESP_OK(nvs_erase_key(handle_1, "foo")); + + TEST_ESP_OK(nvs_get_i8(handle_1, "foo", &v8)); + TEST_ESP_ERR(nvs_get_i32(handle_1, "foo", &v32), ESP_ERR_NVS_NOT_FOUND); + TEST_ESP_OK(nvs_erase_key(handle_1, "foo")); + + TEST_ESP_OK(nvs_get_i8(handle_1, "foo", &v8)); + TEST_ESP_ERR(nvs_get_i32(handle_1, "foo", &v32), ESP_ERR_NVS_NOT_FOUND); + TEST_ESP_OK(nvs_erase_key(handle_1, "foo")); + + TEST_ESP_ERR(nvs_erase_key(handle_1, "foo"), ESP_ERR_NVS_NOT_FOUND); +#else + // New behavior + // Latest nvs_set call replaces any existing value. Only one active value under the key exists in storage + + TEST_ESP_OK(nvs_get_i8(handle_1, "foo", &v8)); + TEST_ESP_ERR(nvs_get_i32(handle_1, "foo", &v32), ESP_ERR_NVS_NOT_FOUND); + TEST_ESP_OK(nvs_erase_key(handle_1, "foo")); + + TEST_ESP_ERR(nvs_get_i8(handle_1, "foo", &v8), ESP_ERR_NVS_NOT_FOUND); + TEST_ESP_ERR(nvs_get_i32(handle_1, "foo", &v32), ESP_ERR_NVS_NOT_FOUND); + TEST_ESP_ERR(nvs_erase_key(handle_1, "foo"), ESP_ERR_NVS_NOT_FOUND); +#endif + + nvs_close(handle_1); + + TEST_ESP_OK(nvs_flash_deinit_partition(NVS_DEFAULT_PART_NAME)); +} + + /* Add new tests above */ /* This test has to be the final one */ diff --git a/components/nvs_flash/host_test/nvs_host_test/sdkconfig.ci.default_set_key b/components/nvs_flash/host_test/nvs_host_test/sdkconfig.ci.default_set_key new file mode 100644 index 0000000000..4ebf01d0f9 --- /dev/null +++ b/components/nvs_flash/host_test/nvs_host_test/sdkconfig.ci.default_set_key @@ -0,0 +1 @@ +# CONFIG_NVS_LEGACY_DUP_KEYS_COMPATIBILITY=n diff --git a/components/nvs_flash/host_test/nvs_host_test/sdkconfig.ci.legacy_set_key b/components/nvs_flash/host_test/nvs_host_test/sdkconfig.ci.legacy_set_key new file mode 100644 index 0000000000..226772f237 --- /dev/null +++ b/components/nvs_flash/host_test/nvs_host_test/sdkconfig.ci.legacy_set_key @@ -0,0 +1 @@ +CONFIG_NVS_LEGACY_DUP_KEYS_COMPATIBILITY=y diff --git a/components/nvs_flash/src/nvs_page.cpp b/components/nvs_flash/src/nvs_page.cpp index 36cde3db25..b69bcfbdaf 100644 --- a/components/nvs_flash/src/nvs_page.cpp +++ b/components/nvs_flash/src/nvs_page.cpp @@ -878,7 +878,7 @@ esp_err_t Page::findItem(uint8_t nsIndex, ItemType datatype, const char* key, si end = ENTRY_COUNT; } - if (nsIndex != NS_ANY && datatype != ItemType::ANY && key != NULL) { + if (nsIndex != NS_ANY && key != NULL) { size_t cachedIndex = mHashList.find(start, Item(nsIndex, datatype, 0, key, chunkIdx)); if (cachedIndex < ENTRY_COUNT) { start = cachedIndex; diff --git a/components/nvs_flash/src/nvs_storage.cpp b/components/nvs_flash/src/nvs_storage.cpp index 48c8a20fc0..5153ae7447 100644 --- a/components/nvs_flash/src/nvs_storage.cpp +++ b/components/nvs_flash/src/nvs_storage.cpp @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2015-2022 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2015-2023 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -285,13 +285,27 @@ esp_err_t Storage::writeItem(uint8_t nsIndex, ItemType datatype, const char* key } Page* findPage = nullptr; + bool matchedTypePageFound = false; Item item; esp_err_t err; if (datatype == ItemType::BLOB) { err = findItem(nsIndex, ItemType::BLOB_IDX, key, findPage, item); + if(err == ESP_OK && findPage != nullptr) { + 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 && findPage != nullptr && datatype == item.datatype) { + matchedTypePageFound = true; + } +#endif } if (err != ESP_OK && err != ESP_ERR_NVS_NOT_FOUND) { @@ -301,7 +315,7 @@ esp_err_t Storage::writeItem(uint8_t nsIndex, ItemType datatype, const char* key if (datatype == ItemType::BLOB) { VerOffset prevStart, nextStart; prevStart = nextStart = VerOffset::VER_0_OFFSET; - if (findPage) { + if (matchedTypePageFound) { // 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. @@ -335,7 +349,7 @@ esp_err_t Storage::writeItem(uint8_t nsIndex, ItemType datatype, const char* key return err; } - if (findPage) { + if (matchedTypePageFound) { /* Erase the blob with earlier version*/ err = eraseMultiPageBlob(nsIndex, key, prevStart); @@ -358,7 +372,7 @@ esp_err_t Storage::writeItem(uint8_t nsIndex, ItemType datatype, const char* key // 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 && + if (matchedTypePageFound && findPage->cmpItem(nsIndex, datatype, key, data, dataSize) == ESP_OK) { return ESP_OK; } @@ -392,12 +406,20 @@ esp_err_t Storage::writeItem(uint8_t nsIndex, ItemType datatype, const char* key 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) { 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) { return ESP_ERR_NVS_REMOVE_FAILED; } diff --git a/docs/en/api-reference/storage/nvs_flash.rst b/docs/en/api-reference/storage/nvs_flash.rst index 8e0bec9db6..c25316c902 100644 --- a/docs/en/api-reference/storage/nvs_flash.rst +++ b/docs/en/api-reference/storage/nvs_flash.rst @@ -35,12 +35,9 @@ NVS operates on key-value pairs. Keys are ASCII strings; the maximum key length Additional types, such as ``float`` and ``double`` might be added later. -Keys are required to be unique. Assigning a new value to an existing key works as follows: +Keys are required to be unique. Assigning a new value to an existing key replaces the old value and data type with the value and data type specified by a write operation. -- If the new value is of the same type as the old one, value is updated. -- If the new value has a different data type, an error is returned. - -Data type check is also performed when reading a value. An error is returned if the data type of the read operation does not match the data type of the value. +A data type check is performed when reading a value. An error is returned if the data type expected by read operation does not match the data type of entry found for the key provided. Namespaces diff --git a/docs/zh_CN/api-reference/storage/nvs_flash.rst b/docs/zh_CN/api-reference/storage/nvs_flash.rst index fd494f1bee..8f05d495d5 100644 --- a/docs/zh_CN/api-reference/storage/nvs_flash.rst +++ b/docs/zh_CN/api-reference/storage/nvs_flash.rst @@ -35,12 +35,9 @@ NVS 的操作对象为键值对,其中键是 ASCII 字符串,当前支持的 后续可能会增加对 ``float`` 和 ``double`` 等其他类型数据的支持。 -键必须唯一。为现有的键写入新的值可能产生如下结果: +键必须唯一。为现有的键写入新值时,会将旧的值及数据类型更新为写入操作指定的值和数据类型。 -- 如果新旧值数据类型相同,则更新值; -- 如果新旧值数据类型不同,则返回错误。 - -读取值时也会执行数据类型检查。如果读取操作的数据类型与该值的数据类型不匹配,则返回错误。 +读取值时会执行数据类型检查。如果读取操作预期的数据类型与对应键的数据类型不匹配,则返回错误。 命名空间