alex/esp-idf
alex/esp-idf
1
0
Fork 0
mirror of https://github.com/espressif/esp-idf.git synced 2024-10-05 20:47:46 -04:00
Code Issues Actions 2 Packages Projects Releases Wiki Activity

Merge branch 'bugfix/nvs_set_datatype' into 'master'

Browse Source 
bugfix(nvs_flash) : fixed nvs_set functions behaviour when called sequentially with same key and different data type(s)

Closes IDFGH-9727

See merge request espressif/esp-idf!25581
This commit is contained in:
Martin Vychodil 2023-10-03 02:21:23 +08:00
commit c50dfa2374
11 changed files with 121 additions and 34 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
components/nvs_flash/.build-test-rules.yml Normal file
View File

@ -0,0 +1,3 @@
components/nvs_flash/host_test:
enable:
- if: IDF_TARGET == "linux"

10
components/nvs_flash/Kconfig
View File

@ -25,4 +25,14 @@ 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 option will switch the nvs_set() family of functions to the legacy mode:
when called repeatedly with the same key but different data type, the existing value
in the NVS remains active and the new value is just stored, actually not accessible through
corresponding nvs_get() call for the key given. Use this option only when your application
relies on such NVS API behaviour.
endmenu

65
components/nvs_flash/host_test/nvs_host_test/main/test_nvs.cpp
View File

@ -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 */

1
components/nvs_flash/host_test/nvs_host_test/sdkconfig.ci.default_set_key Normal file
View File

@ -0,0 +1 @@
# CONFIG_NVS_LEGACY_DUP_KEYS_COMPATIBILITY=n

1
components/nvs_flash/host_test/nvs_host_test/sdkconfig.ci.legacy_set_key Normal file
View File

@ -0,0 +1 @@
CONFIG_NVS_LEGACY_DUP_KEYS_COMPATIBILITY=y

28
components/nvs_flash/host_test/nvs_page_test/main/nvs_page_test.cpp
View File

@ -1,16 +1,8 @@
// Copyright 2015-2016 Espressif Systems (Shanghai) PTE LTD
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
/*
* SPDX-FileCopyrightText: 2015-2023 Espressif Systems (Shanghai) CO LTD
*
* SPDX-License-Identifier: Apache-2.0
*/
#include <stdio.h>
#include "unity.h"
#include "test_fixtures.hpp"
@ -863,7 +855,7 @@ void test_Page_calcEntries__uninit()
NVSPageFixture fix;
TEST_ASSERT_EQUAL(Page::PageState::UNINITIALIZED, fix.page.state());
nvs_stats_t nvsStats = {0, 0, 0, 0};
nvs_stats_t nvsStats = {0, 0, 0, 0, 0};
TEST_ASSERT_EQUAL(ESP_OK, fix.page.calcEntries(nvsStats));
TEST_ASSERT_EQUAL(0, nvsStats.used_entries);
@ -888,7 +880,7 @@ void test_Page_calcEntries__corrupt()
TEST_ASSERT_EQUAL(Page::PageState::CORRUPT, page.state());
nvs_stats_t nvsStats = {0, 0, 0, 0};
nvs_stats_t nvsStats = {0, 0, 0, 0, 0};
TEST_ASSERT_EQUAL(ESP_OK, page.calcEntries(nvsStats));
TEST_ASSERT_EQUAL(0, nvsStats.used_entries);
@ -901,7 +893,7 @@ void test_Page_calcEntries__active_wo_blob()
{
NVSValidPageFixture fix;
nvs_stats_t nvsStats = {0, 0, 0, 0};
nvs_stats_t nvsStats = {0, 0, 0, 0, 0};
TEST_ASSERT_EQUAL(ESP_OK, fix.page.calcEntries(nvsStats));
TEST_ASSERT_EQUAL(2, nvsStats.used_entries);
@ -914,7 +906,7 @@ void test_Page_calcEntries__active_with_blob()
{
NVSValidBlobPageFixture fix;
nvs_stats_t nvsStats = {0, 0, 0, 0};
nvs_stats_t nvsStats = {0, 0, 0, 0, 0};
TEST_ASSERT_EQUAL(ESP_OK, fix.page.calcEntries(nvsStats));
TEST_ASSERT_EQUAL(4, nvsStats.used_entries);
@ -927,7 +919,7 @@ void test_Page_calcEntries__invalid()
{
Page page;
nvs_stats_t nvsStats = {0, 0, 0, 0};
nvs_stats_t nvsStats = {0, 0, 0, 0, 0};
TEST_ASSERT_EQUAL(Page::PageState::INVALID, page.state());

2
components/nvs_flash/src/nvs_page.cpp
View File

@ -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;

30
components/nvs_flash/src/nvs_storage.cpp
View File

@ -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) {
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 && 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;
}

7
docs/en/api-reference/storage/nvs_flash.rst
View File

@ -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

7
docs/zh_CN/api-reference/storage/nvs_flash.rst
View File

@ -35,12 +35,9 @@ NVS 的操作对象为键值对，其中键是 ASCII 字符串，当前支持的
后续可能会增加对 ``float`` 和 ``double`` 等其他类型数据的支持。
键必须唯一。为现有的键写入新的值可能产生如下结果：
键必须唯一。为现有的键写入新值时，会将旧的值及数据类型更新为写入操作指定的值和数据类型。
- 如果新旧值数据类型相同，则更新值；
- 如果新旧值数据类型不同，则返回错误。
读取值时也会执行数据类型检查。如果读取操作的数据类型与该值的数据类型不匹配，则返回错误。
读取值时会执行数据类型检查。如果读取操作预期的数据类型与对应键的数据类型不匹配，则返回错误。
命名空间

1
tools/ci/check_copyright_ignore.txt
View File

@ -586,7 +586,6 @@ components/mbedtls/port/include/sha512_alt.h
components/mbedtls/port/sha/dma/include/esp_sha_dma_priv.h
components/mbedtls/port/sha/dma/sha.c
components/mbedtls/port/sha/parallel_engine/sha.c
components/nvs_flash/host_test/nvs_page_test/main/nvs_page_test.cpp
components/nvs_flash/include/nvs_handle.hpp
components/nvs_flash/src/nvs_cxx_api.cpp
components/nvs_flash/src/nvs_encrypted_partition.hpp