From ad184e979aea71a34b0136c610ec68e6653f8d29 Mon Sep 17 00:00:00 2001 From: Jakob Hasse Date: Wed, 11 May 2022 18:49:55 +0800 Subject: [PATCH] refactor (nvs)!: New interface for iterator functions Closes https://github.com/espressif/esp-idf/issues/7826 * nvs_entry_find(), nvs_entry_next() and nvs_entry_info() return error codes now * nvs_entry_find() and nvs_entry_next() access/modify iterator via parameters, instead of returning an new iterator. Added appropriate documentation in Chinese and English --- components/esp_common/src/esp_err_to_name.c | 5 +- components/nvs_flash/include/nvs.h | 109 ++++++---- components/nvs_flash/src/nvs_api.cpp | 58 ++++-- .../nvs_flash/test_nvs_host/test_nvs.cpp | 192 +++++++++++++----- docs/en/api-reference/storage/nvs_flash.rst | 7 +- docs/en/migration-guides/storage.rst | 49 +++++ .../zh_CN/api-reference/storage/nvs_flash.rst | 11 +- .../advanced/components/cmd_nvs/cmd_nvs.c | 21 +- tools/ci/check_copyright_ignore.txt | 1 - 9 files changed, 322 insertions(+), 131 deletions(-) diff --git a/components/esp_common/src/esp_err_to_name.c b/components/esp_common/src/esp_err_to_name.c index 90733e0c7b..b38638f76e 100644 --- a/components/esp_common/src/esp_err_to_name.c +++ b/components/esp_common/src/esp_err_to_name.c @@ -132,8 +132,9 @@ static const esp_err_msg_t esp_err_msg_table[] = { ERR_TBL_IT(ESP_ERR_NVS_NOT_INITIALIZED), /* 4353 0x1101 The storage driver is not initialized */ # endif # ifdef ESP_ERR_NVS_NOT_FOUND - ERR_TBL_IT(ESP_ERR_NVS_NOT_FOUND), /* 4354 0x1102 Id namespace doesn’t exist yet and mode - is NVS_READONLY */ + ERR_TBL_IT(ESP_ERR_NVS_NOT_FOUND), /* 4354 0x1102 A requested entry couldn't be found or + namespace doesn’t exist yet and mode is + NVS_READONLY */ # endif # ifdef ESP_ERR_NVS_TYPE_MISMATCH ERR_TBL_IT(ESP_ERR_NVS_TYPE_MISMATCH), /* 4355 0x1103 The type of set or get operation doesn't diff --git a/components/nvs_flash/include/nvs.h b/components/nvs_flash/include/nvs.h index 0faef98fd3..c49a212749 100644 --- a/components/nvs_flash/include/nvs.h +++ b/components/nvs_flash/include/nvs.h @@ -28,7 +28,7 @@ typedef nvs_handle_t nvs_handle IDF_DEPRECATED("Replace with nvs_handle_t"); #define ESP_ERR_NVS_BASE 0x1100 /*!< Starting number of error codes */ #define ESP_ERR_NVS_NOT_INITIALIZED (ESP_ERR_NVS_BASE + 0x01) /*!< The storage driver is not initialized */ -#define ESP_ERR_NVS_NOT_FOUND (ESP_ERR_NVS_BASE + 0x02) /*!< Id namespace doesn’t exist yet and mode is NVS_READONLY */ +#define ESP_ERR_NVS_NOT_FOUND (ESP_ERR_NVS_BASE + 0x02) /*!< A requested entry couldn't be found or namespace doesn’t exist yet and mode is NVS_READONLY */ #define ESP_ERR_NVS_TYPE_MISMATCH (ESP_ERR_NVS_BASE + 0x03) /*!< The type of set or get operation doesn't match the type of value stored in NVS */ #define ESP_ERR_NVS_READ_ONLY (ESP_ERR_NVS_BASE + 0x04) /*!< Storage handle was opened as read only */ #define ESP_ERR_NVS_NOT_ENOUGH_SPACE (ESP_ERR_NVS_BASE + 0x05) /*!< There is not enough space in the underlying storage to save the value */ @@ -58,7 +58,7 @@ typedef nvs_handle_t nvs_handle IDF_DEPRECATED("Replace with nvs_handle_t"); #define NVS_DEFAULT_PART_NAME "nvs" /*!< Default partition name of the NVS partition in the partition table */ #define NVS_PART_NAME_MAX_SIZE 16 /*!< maximum length of partition name (excluding null terminator) */ -#define NVS_KEY_NAME_MAX_SIZE 16 /*!< Maximal length of NVS key name (including null terminator) */ +#define NVS_KEY_NAME_MAX_SIZE 16 /*!< Maximum length of NVS key name (including null terminator) */ /** * @brief Mode of opening the non-volatile storage @@ -115,12 +115,12 @@ typedef struct nvs_opaque_iterator_t *nvs_iterator_t; * The default NVS partition is the one that is labelled "nvs" in the partition * table. * - * @param[in] name Namespace name. Maximal length is (NVS_KEY_NAME_MAX_SIZE-1) characters. Shouldn't be empty. - * @param[in] open_mode NVS_READWRITE or NVS_READONLY. If NVS_READONLY, will - * open a handle for reading only. All write requests will - * be rejected for this handle. - * @param[out] out_handle If successful (return code is zero), handle will be - * returned in this argument. + * @param[in] namespace_name Namespace name. Maximum length is (NVS_KEY_NAME_MAX_SIZE-1) characters. Shouldn't be empty. + * @param[in] open_mode NVS_READWRITE or NVS_READONLY. If NVS_READONLY, will + * open a handle for reading only. All write requests will + * be rejected for this handle. + * @param[out] out_handle If successful (return code is zero), handle will be + * returned in this argument. * * @return * - ESP_OK if storage handle was opened successfully @@ -134,7 +134,7 @@ typedef struct nvs_opaque_iterator_t *nvs_iterator_t; * - ESP_ERR_NO_MEM in case memory could not be allocated for the internal structures * - other error codes from the underlying storage driver */ -esp_err_t nvs_open(const char* name, nvs_open_mode_t open_mode, nvs_handle_t *out_handle); +esp_err_t nvs_open(const char* namespace_name, nvs_open_mode_t open_mode, nvs_handle_t *out_handle); /** * @brief Open non-volatile storage with a given namespace from specified partition @@ -143,13 +143,13 @@ esp_err_t nvs_open(const char* name, nvs_open_mode_t open_mode, nvs_handle_t *ou * partition instead of default NVS partition. Note that the specified partition must be registered * with NVS using nvs_flash_init_partition() API. * - * @param[in] part_name Label (name) of the partition of interest for object read/write/erase - * @param[in] name Namespace name. Maximal length is (NVS_KEY_NAME_MAX_SIZE-1) characters. Shouldn't be empty. - * @param[in] open_mode NVS_READWRITE or NVS_READONLY. If NVS_READONLY, will - * open a handle for reading only. All write requests will - * be rejected for this handle. - * @param[out] out_handle If successful (return code is zero), handle will be - * returned in this argument. + * @param[in] part_name Label (name) of the partition of interest for object read/write/erase + * @param[in] namespace_name Namespace name. Maximum length is (NVS_KEY_NAME_MAX_SIZE-1) characters. Shouldn't be empty. + * @param[in] open_mode NVS_READWRITE or NVS_READONLY. If NVS_READONLY, will + * open a handle for reading only. All write requests will + * be rejected for this handle. + * @param[out] out_handle If successful (return code is zero), handle will be + * returned in this argument. * * @return * - ESP_OK if storage handle was opened successfully @@ -163,7 +163,7 @@ esp_err_t nvs_open(const char* name, nvs_open_mode_t open_mode, nvs_handle_t *ou * - ESP_ERR_NO_MEM in case memory could not be allocated for the internal structures * - other error codes from the underlying storage driver */ -esp_err_t nvs_open_from_partition(const char *part_name, const char* name, nvs_open_mode_t open_mode, nvs_handle_t *out_handle); +esp_err_t nvs_open_from_partition(const char *part_name, const char* namespace_name, nvs_open_mode_t open_mode, nvs_handle_t *out_handle); /**@{*/ /** @@ -174,7 +174,7 @@ esp_err_t nvs_open_from_partition(const char *part_name, const char* name, nvs_o * * @param[in] handle Handle obtained from nvs_open function. * Handles that were opened read only cannot be used. - * @param[in] key Key name. Maximal length is (NVS_KEY_NAME_MAX_SIZE-1) characters. Shouldn't be empty. + * @param[in] key Key name. Maximum length is (NVS_KEY_NAME_MAX_SIZE-1) characters. Shouldn't be empty. * @param[in] value The value to set. * * @return @@ -250,7 +250,7 @@ esp_err_t nvs_set_u64 (nvs_handle_t handle, const char* key, uint64_t value); * * @param[in] handle Handle obtained from nvs_open function. * Handles that were opened read only cannot be used. - * @param[in] key Key name. Maximal length is (NVS_KEY_NAME_MAX_SIZE-1) characters. Shouldn't be empty. + * @param[in] key Key name. Maximum length is (NVS_KEY_NAME_MAX_SIZE-1) characters. Shouldn't be empty. * @param[in] value The value to set. * For strings, the maximum length (including null character) is * 4000 bytes, if there is one complete page free for writing. @@ -280,7 +280,7 @@ esp_err_t nvs_set_str (nvs_handle_t handle, const char* key, const char* value); * * @param[in] handle Handle obtained from nvs_open function. * Handles that were opened read only cannot be used. - * @param[in] key Key name. Maximal length is (NVS_KEY_NAME_MAX_SIZE-1) characters. Shouldn't be empty. + * @param[in] key Key name. Maximum length is (NVS_KEY_NAME_MAX_SIZE-1) characters. Shouldn't be empty. * @param[in] value The value to set. * @param[in] length length of binary value to set, in bytes; Maximum length is * 508000 bytes or (97.6% of the partition size - 4000) bytes @@ -326,7 +326,7 @@ esp_err_t nvs_set_blob(nvs_handle_t handle, const char* key, const void* value, * \endcode * * @param[in] handle Handle obtained from nvs_open function. - * @param[in] key Key name. Maximal length is (NVS_KEY_NAME_MAX_SIZE-1) characters. Shouldn't be empty. + * @param[in] key Key name. Maximum length is (NVS_KEY_NAME_MAX_SIZE-1) characters. Shouldn't be empty. * @param out_value Pointer to the output value. * May be NULL for nvs_get_str and nvs_get_blob, in this * case required length will be returned in length argument. @@ -430,7 +430,7 @@ esp_err_t nvs_get_u64 (nvs_handle_t handle, const char* key, uint64_t* out_value * \endcode * * @param[in] handle Handle obtained from nvs_open function. - * @param[in] key Key name. Maximal length is (NVS_KEY_NAME_MAX_SIZE-1) characters. Shouldn't be empty. + * @param[in] key Key name. Maximum length is (NVS_KEY_NAME_MAX_SIZE-1) characters. Shouldn't be empty. * @param[out] out_value Pointer to the output value. * May be NULL for nvs_get_str and nvs_get_blob, in this * case required length will be returned in length argument. @@ -467,7 +467,7 @@ esp_err_t nvs_get_blob(nvs_handle_t handle, const char* key, void* out_value, si * @param[in] handle Storage handle obtained with nvs_open. * Handles that were opened read only cannot be used. * - * @param[in] key Key name. Maximal length is (NVS_KEY_NAME_MAX_SIZE-1) characters. Shouldn't be empty. + * @param[in] key Key name. Maximum length is (NVS_KEY_NAME_MAX_SIZE-1) characters. Shouldn't be empty. * * @return * - ESP_OK if erase operation was successful @@ -618,17 +618,15 @@ esp_err_t nvs_get_used_entry_count(nvs_handle_t handle, size_t* used_entries); * * \code{c} * // Example of listing all the key-value pairs of any type under specified partition and namespace - * nvs_iterator_t it = nvs_entry_find(partition, namespace, NVS_TYPE_ANY); - * while (it != NULL) { - * nvs_entry_info_t info; - * nvs_entry_info(it, &info); - * it = nvs_entry_next(it); - * printf("key '%s', type '%d' \n", info.key, info.type); - * }; - * // Note: no need to release iterator obtained from nvs_entry_find function when - * // nvs_entry_find or nvs_entry_next function return NULL, indicating no other - * // element for specified criteria was found. - * } + * nvs_iterator_t it = NULL; + * esp_err_t res = nvs_entry_find(, , NVS_TYPE_ANY, &it); + * while(res == ESP_OK) { + * nvs_entry_info_t info; + * nvs_entry_info(it, &info); // Can omit error check if parameters are guaranteed to be non-NULL + * printf("key '%s', type '%d' \n", info.key, info.type); + * res = nvs_entry_next(&it); + * } + * nvs_release_iterator(it); * \endcode * * @param[in] part_name Partition name @@ -638,34 +636,55 @@ esp_err_t nvs_get_used_entry_count(nvs_handle_t handle, size_t* used_entries); * * @param[in] type One of nvs_type_t values. * + * @param[out] output_iterator + * Set to a valid iterator to enumerate all the entries found. + * Set to NULL if no entry for specified criteria was found. + * If any other error except ESP_ERR_INVALID_ARG occurs, \c output_iterator is NULL, too. + * If ESP_ERR_INVALID_ARG occurs, \c output_iterator is not changed. + * If a valid iterator is obtained through this function, it has to be released + * using \c nvs_release_iterator when not used any more, unless ESP_ERR_INVALID_ARG is returned. + * * @return - * Iterator used to enumerate all the entries found, - * or NULL if no entry satisfying criteria was found. - * Iterator obtained through this function has to be released - * using nvs_release_iterator when not used any more. + * - ESP_OK if no internal error or programming error occurred. + * - ESP_ERR_NVS_NOT_FOUND if no element of specified criteria has been found. + * - ESP_ERR_NO_MEM if memory has been exhausted during allocation of internal structures. + * - ESP_ERR_INVALID_ARG if any of the parameters is NULL. + * Note: don't release \c output_iterator in case ESP_ERR_INVALID_ARG has been returned */ -nvs_iterator_t nvs_entry_find(const char *part_name, const char *namespace_name, nvs_type_t type); +esp_err_t nvs_entry_find(const char *part_name, + const char *namespace_name, + nvs_type_t type, + nvs_iterator_t *output_iterator); /** - * @brief Returns next item matching the iterator criteria, NULL if no such item exists. + * @brief Advances the iterator to next item matching the iterator criteria. * * Note that any copies of the iterator will be invalid after this call. * - * @param[in] iterator Iterator obtained from nvs_entry_find function. Must be non-NULL. + * @param[inout] iterator Iterator obtained from nvs_entry_find function. Must be non-NULL. + * If any error except ESP_ERR_INVALID_ARG occurs, \c iterator is set to NULL. + * If ESP_ERR_INVALID_ARG occurs, \c iterator is not changed. * * @return - * NULL if no entry was found, valid nvs_iterator_t otherwise. + * - ESP_OK if no internal error or programming error occurred. + * - ESP_ERR_NVS_NOT_FOUND if no next element matching the iterator criteria. + * - ESP_ERR_INVALID_ARG if \c iterator is NULL. + * - Possibly other errors in the future for internal programming or flash errors. */ -nvs_iterator_t nvs_entry_next(nvs_iterator_t iterator); +esp_err_t nvs_entry_next(nvs_iterator_t *iterator); /** * @brief Fills nvs_entry_info_t structure with information about entry pointed to by the iterator. * - * @param[in] iterator Iterator obtained from nvs_entry_find or nvs_entry_next function. Must be non-NULL. + * @param[in] iterator Iterator obtained from nvs_entry_find function. Must be non-NULL. * * @param[out] out_info Structure to which entry information is copied. + * + * @return + * - ESP_OK if all parameters are valid; current iterator data has been written to out_info + * - ESP_ERR_INVALID_ARG if one of the parameters is NULL. */ -void nvs_entry_info(nvs_iterator_t iterator, nvs_entry_info_t *out_info); +esp_err_t nvs_entry_info(const nvs_iterator_t iterator, nvs_entry_info_t *out_info); /** * @brief Release iterator diff --git a/components/nvs_flash/src/nvs_api.cpp b/components/nvs_flash/src/nvs_api.cpp index 866bca53f9..706c22a864 100644 --- a/components/nvs_flash/src/nvs_api.cpp +++ b/components/nvs_flash/src/nvs_api.cpp @@ -269,13 +269,13 @@ static esp_err_t nvs_find_ns_handle(nvs_handle_t c_handle, NVSHandleSimple** han return ESP_OK; } -extern "C" esp_err_t nvs_open_from_partition(const char *part_name, const char* name, nvs_open_mode_t open_mode, nvs_handle_t *out_handle) +extern "C" esp_err_t nvs_open_from_partition(const char *part_name, const char* namespace_name, nvs_open_mode_t open_mode, nvs_handle_t *out_handle) { Lock lock; - ESP_LOGD(TAG, "%s %s %d", __func__, name, open_mode); + ESP_LOGD(TAG, "%s %s %d", __func__, namespace_name, open_mode); NVSHandleSimple *handle; - esp_err_t result = NVSPartitionManager::get_instance()->open_handle(part_name, name, open_mode, &handle); + esp_err_t result = NVSPartitionManager::get_instance()->open_handle(part_name, namespace_name, open_mode, &handle); if (result == ESP_OK) { NVSHandleEntry *entry = new (std::nothrow) NVSHandleEntry(handle, part_name); if (entry) { @@ -290,9 +290,9 @@ extern "C" esp_err_t nvs_open_from_partition(const char *part_name, const char* return result; } -extern "C" esp_err_t nvs_open(const char* name, nvs_open_mode_t open_mode, nvs_handle_t *out_handle) +extern "C" esp_err_t nvs_open(const char* namespace_name, nvs_open_mode_t open_mode, nvs_handle_t *out_handle) { - return nvs_open_from_partition(NVS_DEFAULT_PART_NAME, name, open_mode, out_handle); + return nvs_open_from_partition(NVS_DEFAULT_PART_NAME, namespace_name, open_mode, out_handle); } extern "C" void nvs_close(nvs_handle_t handle) @@ -718,47 +718,71 @@ static nvs_iterator_t create_iterator(nvs::Storage *storage, nvs_type_t type) return it; } -extern "C" nvs_iterator_t nvs_entry_find(const char *part_name, const char *namespace_name, nvs_type_t type) +// In case of errors except for parameter error, output_iterator is set to nullptr to make releasing iterators easier +extern "C" esp_err_t nvs_entry_find(const char *part_name, const char *namespace_name, nvs_type_t type, nvs_iterator_t *output_iterator) { + if (part_name == nullptr || output_iterator == nullptr) { + return ESP_ERR_INVALID_ARG; + } + + esp_err_t lock_result = Lock::init(); + if (lock_result != ESP_OK) { + *output_iterator = nullptr; + return lock_result; + } Lock lock; nvs::Storage *pStorage; pStorage = lookup_storage_from_name(part_name); if (pStorage == nullptr) { - return nullptr; + *output_iterator = nullptr; + return ESP_ERR_NVS_NOT_FOUND; } nvs_iterator_t it = create_iterator(pStorage, type); if (it == nullptr) { - return nullptr; + *output_iterator = nullptr; + return ESP_ERR_NO_MEM; } bool entryFound = pStorage->findEntry(it, namespace_name); if (!entryFound) { free(it); - return nullptr; + *output_iterator = nullptr; + return ESP_ERR_NVS_NOT_FOUND; } - return it; + *output_iterator = it; + return ESP_OK; } -extern "C" nvs_iterator_t nvs_entry_next(nvs_iterator_t it) +extern "C" esp_err_t nvs_entry_next(nvs_iterator_t *iterator) { + if (iterator == nullptr) { + return ESP_ERR_INVALID_ARG; + } + Lock lock; - NVS_ASSERT_OR_RETURN(it, nullptr); - bool entryFound = it->storage->nextEntry(it); + bool entryFound = (*iterator)->storage->nextEntry(*iterator); if (!entryFound) { - free(it); - return nullptr; + free(*iterator); + *iterator = nullptr; + return ESP_ERR_NVS_NOT_FOUND; } - return it; + return ESP_OK; } -extern "C" void nvs_entry_info(nvs_iterator_t it, nvs_entry_info_t *out_info) +extern "C" esp_err_t nvs_entry_info(const nvs_iterator_t it, nvs_entry_info_t *out_info) { + if (it == nullptr || out_info == nullptr) { + return ESP_ERR_INVALID_ARG; + } + *out_info = it->entry_info; + + return ESP_OK; } extern "C" void nvs_release_iterator(nvs_iterator_t it) diff --git a/components/nvs_flash/test_nvs_host/test_nvs.cpp b/components/nvs_flash/test_nvs_host/test_nvs.cpp index 76affc310d..c97ba77960 100644 --- a/components/nvs_flash/test_nvs_host/test_nvs.cpp +++ b/components/nvs_flash/test_nvs_host/test_nvs.cpp @@ -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-2022 Espressif Systems (Shanghai) CO LTD + * + * SPDX-License-Identifier: Apache-2.0 + */ #include "catch.hpp" #include "nvs.hpp" #include "nvs_test_api.h" @@ -738,6 +730,49 @@ TEST_CASE("deinit partition doesn't affect other partition's open handles", "[nv TEST_ESP_OK(nvs_flash_deinit_partition(OTHER_PARTITION_NAME)); } +TEST_CASE("nvs iterator nvs_entry_find invalid parameter test", "[nvs]") +{ + nvs_iterator_t it = reinterpret_cast(0xbeef); + CHECK(nvs_entry_find(nullptr, NULL, NVS_TYPE_ANY, &it) == ESP_ERR_INVALID_ARG); + CHECK(nvs_entry_find("nvs", NULL, NVS_TYPE_ANY, nullptr) == ESP_ERR_INVALID_ARG); +} + +TEST_CASE("nvs iterator nvs_entry_find doesn't change iterator on parameter error", "[nvs]") +{ + nvs_iterator_t it = reinterpret_cast(0xbeef); + REQUIRE(nvs_entry_find(nullptr, NULL, NVS_TYPE_ANY, &it) == ESP_ERR_INVALID_ARG); + CHECK(it == reinterpret_cast(0xbeef)); + + it = nullptr; + REQUIRE(nvs_entry_find(nullptr, NULL, NVS_TYPE_ANY, &it) == ESP_ERR_INVALID_ARG); + CHECK(it == nullptr); +} + +TEST_CASE("nvs_entry_next return ESP_ERR_INVALID_ARG on parameter is NULL", "[nvs]") +{ + CHECK(nvs_entry_next(nullptr) == ESP_ERR_INVALID_ARG); +} + +TEST_CASE("nvs_entry_info fails with ESP_ERR_INVALID_ARG if a parameter is NULL", "[nvs]") +{ + nvs_iterator_t it = reinterpret_cast(0xbeef); + nvs_entry_info_t info; + CHECK(nvs_entry_info(it, nullptr) == ESP_ERR_INVALID_ARG); + CHECK(nvs_entry_info(nullptr, &info) == ESP_ERR_INVALID_ARG); +} + +TEST_CASE("nvs_entry_info doesn't change iterator on parameter error", "[nvs]") +{ + nvs_iterator_t it = reinterpret_cast(0xbeef); + nvs_entry_info_t info; + REQUIRE(nvs_entry_info(it, nullptr) == ESP_ERR_INVALID_ARG); + CHECK(it == reinterpret_cast(0xbeef)); + + it = nullptr; + REQUIRE(nvs_entry_info(it, nullptr) == ESP_ERR_INVALID_ARG); + CHECK(it == nullptr); +} + TEST_CASE("nvs iterators tests", "[nvs]") { PartitionEmulationFixture f(0, 5); @@ -780,30 +815,76 @@ TEST_CASE("nvs iterators tests", "[nvs]") TEST_ESP_OK(nvs_set_u64(handle_2, "value4", 555)); auto entry_count = [](const char *part, const char *name, nvs_type_t type)-> int { - int count; - nvs_iterator_t it = nvs_entry_find(part, name, type); - for (count = 0; it != nullptr; count++) { - it = nvs_entry_next(it); + int count = 0; + nvs_iterator_t it = nullptr; + esp_err_t res = nvs_entry_find(part, name, type, &it); + for (count = 0; res == ESP_OK; count++) { + res = nvs_entry_next(&it); } + CHECK(res == ESP_ERR_NVS_NOT_FOUND); // after finishing the loop or if no entry was found to begin with, + // res has to be ESP_ERR_NVS_NOT_FOUND or some internal error + // or programming error occurred + nvs_release_iterator(it); // unneccessary call but emphasizes the programming pattern return count; }; - SECTION("Number of entries found for specified namespace and type is correct") - { - CHECK(nvs_entry_find("", NULL, NVS_TYPE_ANY) == NULL); - CHECK(entry_count(NVS_DEFAULT_PART_NAME, NULL, NVS_TYPE_ANY) == 15); - CHECK(entry_count(NVS_DEFAULT_PART_NAME, name_1, NVS_TYPE_ANY) == 11); - CHECK(entry_count(NVS_DEFAULT_PART_NAME, name_1, NVS_TYPE_I32) == 3); - CHECK(entry_count(NVS_DEFAULT_PART_NAME, NULL, NVS_TYPE_I32) == 5); - CHECK(entry_count(NVS_DEFAULT_PART_NAME, NULL, NVS_TYPE_U64) == 1); - } + SECTION("No partition found return ESP_ERR_NVS_NOT_FOUND") + { + CHECK(nvs_entry_find("", NULL, NVS_TYPE_ANY, &it) == ESP_ERR_NVS_NOT_FOUND); + } - SECTION("New entry is not created when existing key-value pair is set") - { - CHECK(entry_count(NVS_DEFAULT_PART_NAME, name_2, NVS_TYPE_ANY) == 4); - TEST_ESP_OK(nvs_set_i32(handle_2, "value1", -222)); - CHECK(entry_count(NVS_DEFAULT_PART_NAME, name_2, NVS_TYPE_ANY) == 4); - } + SECTION("No matching namespace found return ESP_ERR_NVS_NOT_FOUND") + { + CHECK(nvs_entry_find(NVS_DEFAULT_PART_NAME, "nonexistent", NVS_TYPE_ANY, &it) == ESP_ERR_NVS_NOT_FOUND); + } + + SECTION("nvs_entry_find sets iterator to null if no matching element found") + { + it = reinterpret_cast(0xbeef); + REQUIRE(nvs_entry_find(NVS_DEFAULT_PART_NAME, "nonexistent", NVS_TYPE_I16, &it) == ESP_ERR_NVS_NOT_FOUND); + CHECK(it == nullptr); + } + + SECTION("Finding iterator means iterator is valid") + { + it = nullptr; + CHECK(nvs_entry_find(NVS_DEFAULT_PART_NAME, nullptr, NVS_TYPE_ANY, &it) == ESP_OK); + CHECK(it != nullptr); + nvs_release_iterator(it); + } + + SECTION("Return ESP_ERR_NVS_NOT_FOUND after iterating over last matching element") + { + it = nullptr; + REQUIRE(nvs_entry_find(NVS_DEFAULT_PART_NAME, name_1, NVS_TYPE_I16, &it) == ESP_OK); + REQUIRE(it != nullptr); + CHECK(nvs_entry_next(&it) == ESP_ERR_NVS_NOT_FOUND); + } + + SECTION("Set iterator to NULL after iterating over last matching element") + { + it = nullptr; + REQUIRE(nvs_entry_find(NVS_DEFAULT_PART_NAME, name_1, NVS_TYPE_I16, &it) == ESP_OK); + REQUIRE(it != nullptr); + REQUIRE(nvs_entry_next(&it) == ESP_ERR_NVS_NOT_FOUND); + CHECK(it == nullptr); + } + + SECTION("Number of entries found for specified namespace and type is correct") + { + CHECK(entry_count(NVS_DEFAULT_PART_NAME, NULL, NVS_TYPE_ANY) == 15); + CHECK(entry_count(NVS_DEFAULT_PART_NAME, name_1, NVS_TYPE_ANY) == 11); + CHECK(entry_count(NVS_DEFAULT_PART_NAME, name_1, NVS_TYPE_I32) == 3); + CHECK(entry_count(NVS_DEFAULT_PART_NAME, NULL, NVS_TYPE_I32) == 5); + CHECK(entry_count(NVS_DEFAULT_PART_NAME, NULL, NVS_TYPE_U64) == 1); + } + + SECTION("New entry is not created when existing key-value pair is set") + { + CHECK(entry_count(NVS_DEFAULT_PART_NAME, name_2, NVS_TYPE_ANY) == 4); + TEST_ESP_OK(nvs_set_i32(handle_2, "value1", -222)); + CHECK(entry_count(NVS_DEFAULT_PART_NAME, name_2, NVS_TYPE_ANY) == 4); + } SECTION("Number of entries found decrease when entry is erased") { @@ -814,30 +895,34 @@ TEST_CASE("nvs iterators tests", "[nvs]") SECTION("All fields of nvs_entry_info_t structure are correct") { - it = nvs_entry_find(NVS_DEFAULT_PART_NAME, name_1, NVS_TYPE_I32); - CHECK(it != nullptr); + it = nullptr; + esp_err_t res = nvs_entry_find(NVS_DEFAULT_PART_NAME, name_1, NVS_TYPE_I32, &it); + REQUIRE(res == ESP_OK); string key = "value5"; - do { - nvs_entry_info(it, &info); + while (res == ESP_OK) { + REQUIRE(nvs_entry_info(it, &info) == ESP_OK); CHECK(string(name_1) == info.namespace_name); CHECK(key == info.key); CHECK(info.type == NVS_TYPE_I32); - it = nvs_entry_next(it); + res = nvs_entry_next(&it); key[5]++; - } while (it != NULL); - nvs_release_iterator(it); + } + CHECK(res == ESP_ERR_NVS_NOT_FOUND); // after finishing the loop, res has to be ESP_ERR_NVS_NOT_FOUND + // or some internal error or programming error occurred + CHECK(key == "value8"); + nvs_release_iterator(it); // unneccessary call but emphasizes the programming pattern } SECTION("Entry info is not affected by subsequent erase") { nvs_entry_info_t info_after_erase; - it = nvs_entry_find(NVS_DEFAULT_PART_NAME, name_1, NVS_TYPE_ANY); - nvs_entry_info(it, &info); + CHECK(nvs_entry_find(NVS_DEFAULT_PART_NAME, name_1, NVS_TYPE_ANY, &it) == ESP_OK); + REQUIRE(nvs_entry_info(it, &info) == ESP_OK); TEST_ESP_OK(nvs_erase_key(handle_1, "value1")); - nvs_entry_info(it, &info_after_erase); + REQUIRE(nvs_entry_info(it, &info_after_erase) == ESP_OK); CHECK(memcmp(&info, &info_after_erase, sizeof(info)) == 0); nvs_release_iterator(it); } @@ -846,10 +931,10 @@ TEST_CASE("nvs iterators tests", "[nvs]") { nvs_entry_info_t info_after_set; - it = nvs_entry_find(NVS_DEFAULT_PART_NAME, name_1, NVS_TYPE_ANY); - nvs_entry_info(it, &info); + CHECK(nvs_entry_find(NVS_DEFAULT_PART_NAME, name_1, NVS_TYPE_ANY, &it) == ESP_OK); + REQUIRE(nvs_entry_info(it, &info) == ESP_OK); TEST_ESP_OK(nvs_set_u8(handle_1, info.key, 44)); - nvs_entry_info(it, &info_after_set); + REQUIRE(nvs_entry_info(it, &info_after_set) == ESP_OK); CHECK(memcmp(&info, &info_after_set, sizeof(info)) == 0); nvs_release_iterator(it); } @@ -867,14 +952,17 @@ TEST_CASE("nvs iterators tests", "[nvs]") } int entries_found = 0; - it = nvs_entry_find(NVS_DEFAULT_PART_NAME, name_3, NVS_TYPE_ANY); - while(it != nullptr) { + it = nullptr; + esp_err_t res = nvs_entry_find(NVS_DEFAULT_PART_NAME, name_3, NVS_TYPE_ANY, &it); + while(res == ESP_OK) { entries_found++; - it = nvs_entry_next(it); + res = nvs_entry_next(&it); } + CHECK(res == ESP_ERR_NVS_NOT_FOUND); // after finishing the loop, res has to be ESP_ERR_NVS_NOT_FOUND + // or some internal error or programming error occurred CHECK(entries_created == entries_found); - nvs_release_iterator(it); + nvs_release_iterator(it); // unneccessary call but emphasizes the programming pattern nvs_close(handle_3); } @@ -926,8 +1014,7 @@ TEST_CASE("Iterator with not matching type iterates correctly", "[nvs]") TEST_ESP_OK(nvs_commit(my_handle)); nvs_close(my_handle); - it = nvs_entry_find(NVS_DEFAULT_PART_NAME, NAMESPACE, NVS_TYPE_I32); - CHECK(it == NULL); + CHECK(nvs_entry_find(NVS_DEFAULT_PART_NAME, NAMESPACE, NVS_TYPE_I32, &it) == ESP_ERR_NVS_NOT_FOUND); // re-init to trigger cleaning up of broken items -> a corrupted string will be erased nvs_flash_deinit(); @@ -935,8 +1022,7 @@ TEST_CASE("Iterator with not matching type iterates correctly", "[nvs]") NVS_FLASH_SECTOR, NVS_FLASH_SECTOR_COUNT_MIN)); - it = nvs_entry_find(NVS_DEFAULT_PART_NAME, NAMESPACE, NVS_TYPE_STR); - CHECK(it != NULL); + CHECK(nvs_entry_find(NVS_DEFAULT_PART_NAME, NAMESPACE, NVS_TYPE_STR, &it) == ESP_OK); nvs_release_iterator(it); // without deinit it affects "nvs api tests" diff --git a/docs/en/api-reference/storage/nvs_flash.rst b/docs/en/api-reference/storage/nvs_flash.rst index 00afdbc025..8ac47cab10 100644 --- a/docs/en/api-reference/storage/nvs_flash.rst +++ b/docs/en/api-reference/storage/nvs_flash.rst @@ -55,11 +55,12 @@ Iterators allow to list key-value pairs stored in NVS, based on specified partit There are the following functions available: -- :cpp:func:`nvs_entry_find` returns an opaque handle, which is used in subsequent calls to the :cpp:func:`nvs_entry_next` and :cpp:func:`nvs_entry_info` functions. -- :cpp:func:`nvs_entry_next` returns iterator to the next key-value pair. +- :cpp:func:`nvs_entry_find` creates an opaque handle, which is used in subsequent calls to the :cpp:func:`nvs_entry_next` and :cpp:func:`nvs_entry_info` functions. +- :cpp:func:`nvs_entry_next` advances an iterator to the next key-value pair. - :cpp:func:`nvs_entry_info` returns information about each key-value pair -If none or no other key-value pair was found for given criteria, :cpp:func:`nvs_entry_find` and :cpp:func:`nvs_entry_next` return NULL. In that case, the iterator does not have to be released. If the iterator is no longer needed, you can release it by using the function :cpp:func:`nvs_release_iterator`. +In general, all iterators obtained via :cpp:func:`nvs_entry_find` have to be released using :cpp:func:`nvs_release_iterator`, which also tolerates ``NULL`` iterators. +:cpp:func:`nvs_entry_find` and :cpp:func:`nvs_entry_next` will set the given iterator to ``NULL`` or a valid iterator in all cases except a parameter error occured (i.e., return ``ESP_ERR_NVS_NOT_FOUND``). In case of a parameter error, the given iterator will not be modified. Hence, it is best practice to initialize the iterator to ``NULL`` before calling :cpp:func:`nvs_entry_find` to avoid complicated error checking before releasing the iterator. Security, tampering, and robustness diff --git a/docs/en/migration-guides/storage.rst b/docs/en/migration-guides/storage.rst index fd700ca644..ea1bb06c48 100644 --- a/docs/en/migration-guides/storage.rst +++ b/docs/en/migration-guides/storage.rst @@ -18,3 +18,52 @@ esp_vfs_semihost_register() signature change -------------------------------------------- New signature is ``esp_err_t esp_vfs_semihost_register(const char* base_path);`` Absolute path as a second parameter will no longer in use. Instead, the OpenOCD command ``ESP_SEMIHOST_BASEDIR`` should be used to set the full path on the host. + +NVS +--- + +``nvs_entry_find()``, ``nvs_entry_next()`` and ``nvs_entry_info()`` always return ``esp_err_t`` now instead of ``void`` or ``nvs_iterator_t``. This provides better error reporting when parameters are invalid or something goes wrong internally than returning ``nullptr`` instead of a valid iterator or checking parameters with ``assert``. ``nvs_entry_find()`` and ``nvs_entry_next()`` modify iterators via parameters now instead of returning an iterator. + +The old programming pattern to iterate over an NVS partition was as follows: + +.. highlight:: c + +:: + + nvs_iterator_t it = nvs_entry_find(, , NVS_TYPE_ANY); + while (it != NULL) { + nvs_entry_info_t info; + nvs_entry_info(it, &info); + it = nvs_entry_next(it); + printf("key '%s', type '%d' \n", info.key, info.type); + }; + +The new programming pattern to iterate over an NVS partition is now: + +.. highlight:: c + +:: + + nvs_iterator_t it = nullptr; + esp_err_t res = nvs_entry_find(, , NVS_TYPE_ANY, &it); + while(res == ESP_OK) { + nvs_entry_info_t info; + nvs_entry_info(it, &info); // Can omit error check if parameters are guaranteed to be non-NULL + printf("key '%s', type '%d' \n", info.key, info.type); + res = nvs_entry_next(&it); + } + nvs_release_iterator(it); + +Signature Changes +^^^^^^^^^^^^^^^^^ + +``nvs_iterator_t nvs_entry_find(const char *part_name, const char *namespace_name, nvs_type_t type)`` changes to ``esp_err_t nvs_entry_find(const char *part_name, const char *namespace_name, nvs_type_t type, nvs_iterator_t *output_iterator)``. The iterator is returned via the parameter ``output_iterator`` instead of a return value. This allows reporting additional errors, like e.g. memory errors, via the new return value. + +``nvs_iterator_t nvs_entry_next(nvs_iterator_t iterator)`` changes to ``esp_err_t nvs_entry_next(nvs_iterator_t *it)``. This allows reporting parameter errors and internal errors, like e.g. flash errors. + +``void nvs_entry_info(nvs_iterator_t iterator, nvs_entry_info_t *out_info)`` changes to ``esp_err_t nvs_entry_info(const nvs_iterator_t iterator, nvs_entry_info_t *out_info)`` to allow reporting parameter errors. + +Iterator Validity +^^^^^^^^^^^^^^^^^ + +Note that due to the new signatures, it is possible to have an invalid iterator from ``nvs_entry_find()``, if there is a parameter errors. Hence, it is important to initialize the iterator with ``NULL`` before using ``nvs_entry_find()`` to avoid complex error checking before calling ``nvs_release_iterator()``. A good example is the programming pattern above. diff --git a/docs/zh_CN/api-reference/storage/nvs_flash.rst b/docs/zh_CN/api-reference/storage/nvs_flash.rst index 31fa825329..3025ed1b76 100644 --- a/docs/zh_CN/api-reference/storage/nvs_flash.rst +++ b/docs/zh_CN/api-reference/storage/nvs_flash.rst @@ -55,11 +55,12 @@ NVS 迭代器 您可以使用以下函数，执行相关操作： -- ``nvs_entry_find``：返回一个不透明句柄，用于后续调用 ``nvs_entry_next`` 和 ``nvs_entry_info`` 函数； -- ``nvs_entry_next``：返回指向下一个键值对的迭代器； +- ``nvs_entry_find``：创建一个不透明句柄，用于后续调用 ``nvs_entry_next`` 和 ``nvs_entry_info`` 函数； +- ``nvs_entry_next``：让迭代器指向下一个键值对； - ``nvs_entry_info``：返回每个键值对的信息。 -如果未找到符合标准的键值对，``nvs_entry_find`` 和 ``nvs_entry_next`` 将返回 NULL，此时不必释放迭代器。若不再需要迭代器，可使用 ``nvs_release_iterator`` 释放迭代器。 +总的来说，所有通过 :cpp:func:`nvs_entry_find` 获得的迭代器（包括 ``NULL`` 迭代器）都必须使用 :cpp:func:`nvs_release_iterator` 释放。 +一般情况下，:cpp:func:`nvs_entry_find` 和 :cpp:func:`nvs_entry_next` 会将给定的迭代器设置为 ``NULL`` 或为一个有效的迭代器。但如果出现参数错误（如返回 ``ESP_ERR_NVS_NOT_FOUND``），给定的迭代器不会被修改。因此，在调用 :cpp:func:`nvs_entry_find` 之前最好将迭代器初始化为 ``NULL``，这样可以避免在释放迭代器之前进行复杂的错误检查。 安全性、篡改性及鲁棒性 @@ -168,7 +169,7 @@ ESP-IDF :example:`storage` 目录下提供了数个代码示例： :example:`storage/nvs_rw_blob`　 - 演示如何读取及写入 NVS 单个整数值和 Blob（二进制大对象），并在 NVS 中存储这一数值，即便 {IDF_TARGET_NAME} 模组重启也不会消失。 + 演示如何读取及写入 NVS 单个整数值和 BLOB（二进制大对象），并在 NVS 中存储这一数值，即便 {IDF_TARGET_NAME} 模组重启也不会消失。 * value - 记录 {IDF_TARGET_NAME} 模组软重启次数和硬重启次数。 * blob - 内含记录模组运行次数的表格。此表格将被从 NVS 读取至动态分配的 RAM 上。每次手动软重启后，表格内运行次数即增加一次，新加的运行次数被写入 NVS。下拉 GPIO0 即可手动软重启。 @@ -177,7 +178,7 @@ ESP-IDF :example:`storage` 目录下提供了数个代码示例： :example:`storage/nvs_rw_value_cxx` - 这个例子与 :example:`storage/nvs_rw_value` 完全一样，只是使用了 C++ 的 NVS 处理类。 + 这个例子与 :example:`storage/nvs_rw_value` 完全一样，只是使用了 C++ 的 NVS 句柄类。 内部实现 --------- diff --git a/examples/system/console/advanced/components/cmd_nvs/cmd_nvs.c b/examples/system/console/advanced/components/cmd_nvs/cmd_nvs.c index febd5cad7a..df2382e96f 100644 --- a/examples/system/console/advanced/components/cmd_nvs/cmd_nvs.c +++ b/examples/system/console/advanced/components/cmd_nvs/cmd_nvs.c @@ -374,20 +374,31 @@ static int list(const char *part, const char *name, const char *str_type) { nvs_type_t type = str_to_type(str_type); - nvs_iterator_t it = nvs_entry_find(part, NULL, type); - if (it == NULL) { - ESP_LOGE(TAG, "No such enty was found"); + nvs_iterator_t it = NULL; + esp_err_t result = nvs_entry_find(part, NULL, type, &it); + if (result == ESP_ERR_NVS_NOT_FOUND) { + ESP_LOGE(TAG, "No such entry was found"); + return 1; + } + + if (result != ESP_OK) { + ESP_LOGE(TAG, "NVS error: %s", esp_err_to_name(result)); return 1; } do { nvs_entry_info_t info; nvs_entry_info(it, &info); - it = nvs_entry_next(it); + result = nvs_entry_next(&it); printf("namespace '%s', key '%s', type '%s' \n", info.namespace_name, info.key, type_to_str(info.type)); - } while (it != NULL); + } while (result == ESP_OK); + + if (result != ESP_ERR_NVS_NOT_FOUND) { // the last iteration ran into an internal error + ESP_LOGE(TAG, "NVS error %s at current iteration, stopping.", esp_err_to_name(result)); + return 1; + } return 0; } diff --git a/tools/ci/check_copyright_ignore.txt b/tools/ci/check_copyright_ignore.txt index d358374a2d..199c24665f 100644 --- a/tools/ci/check_copyright_ignore.txt +++ b/tools/ci/check_copyright_ignore.txt @@ -1111,7 +1111,6 @@ components/nvs_flash/test_nvs_host/spi_flash_emulation.cpp components/nvs_flash/test_nvs_host/spi_flash_emulation.h components/nvs_flash/test_nvs_host/test_fixtures.hpp components/nvs_flash/test_nvs_host/test_intrusive_list.cpp -components/nvs_flash/test_nvs_host/test_nvs.cpp components/nvs_flash/test_nvs_host/test_nvs_cxx_api.cpp components/nvs_flash/test_nvs_host/test_nvs_handle.cpp components/nvs_flash/test_nvs_host/test_nvs_initialization.cpp