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

@@ -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
+    ERR_TBL_IT(ESP_ERR_NVS_NOT_FOUND),                          /*  4354 0x1102 A requested entry couldn't be found or
-                                                                                is NVS_READONLY */
+                                                                                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

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,7 +115,7 @@ 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]  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.
@@ -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
@@ -144,7 +144,7 @@ esp_err_t nvs_open(const char* name, nvs_open_mode_t open_mode, nvs_handle_t *ou
  * 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]  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.
@@ -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);
+ *  nvs_iterator_t it = NULL;
- * while (it != NULL) {
+ *  esp_err_t res = nvs_entry_find(<nvs_partition_name>, <namespace>, NVS_TYPE_ANY, &it);
+ *  while(res == ESP_OK) {
  *      nvs_entry_info_t info;
- *         nvs_entry_info(it, &info);
+ *      nvs_entry_info(it, &info); // Can omit error check if parameters are guaranteed to be non-NULL
- *         it = nvs_entry_next(it);
  *      printf("key '%s', type '%d' \n", info.key, info.type);
- * };
+ *      res = nvs_entry_next(&it);
- * // 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_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,
+ *             - ESP_OK if no internal error or programming error occurred.
- *          or NULL if no entry satisfying criteria was found.
+ *             - ESP_ERR_NVS_NOT_FOUND if no element of specified criteria has been found.
- *          Iterator obtained through this function has to be released
+ *             - ESP_ERR_NO_MEM if memory has been exhausted during allocation of internal structures.
- *          using nvs_release_iterator when not used any more.
+ *             - 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

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);
+        free(*iterator);
-        return nullptr;
+        *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)

components/nvs_flash/test_nvs_host/test_nvs.cpp

@@ -1,16 +1,8 @@
-// Copyright 2015-2016 Espressif Systems (Shanghai) PTE LTD
+/*
-//
+ * SPDX-FileCopyrightText: 2015-2022 Espressif Systems (Shanghai) CO LTD
-// Licensed under the Apache License, Version 2.0 (the "License");
+ *
-// you may not use this file except in compliance with the License.
+ * SPDX-License-Identifier: Apache-2.0
-// 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.
 #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<nvs_iterator_t>(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<nvs_iterator_t>(0xbeef);
+    REQUIRE(nvs_entry_find(nullptr, NULL, NVS_TYPE_ANY, &it) == ESP_ERR_INVALID_ARG);
+    CHECK(it == reinterpret_cast<nvs_iterator_t>(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<nvs_iterator_t>(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<nvs_iterator_t>(0xbeef);
+    nvs_entry_info_t info;
+    REQUIRE(nvs_entry_info(it, nullptr) == ESP_ERR_INVALID_ARG);
+    CHECK(it == reinterpret_cast<nvs_iterator_t>(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,17 +815,63 @@ 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;
+        int count = 0;
-        nvs_iterator_t it = nvs_entry_find(part, name, type);
+        nvs_iterator_t it = nullptr;
-        for (count = 0; it != nullptr; count++) {
+        esp_err_t res = nvs_entry_find(part, name, type, &it);
-            it = nvs_entry_next(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("No partition found return ESP_ERR_NVS_NOT_FOUND")
+    {
+        CHECK(nvs_entry_find("", NULL, NVS_TYPE_ANY, &it) == ESP_ERR_NVS_NOT_FOUND);
+    }
+
+    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<nvs_iterator_t>(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(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);
@@ -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);
+        it = nullptr;
-        CHECK(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 {
+        while (res == ESP_OK) {
-            nvs_entry_info(it, &info);
+            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);
+        CHECK(nvs_entry_find(NVS_DEFAULT_PART_NAME, name_1, NVS_TYPE_ANY, &it) == ESP_OK);
-        nvs_entry_info(it, &info);
+        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);
+        CHECK(nvs_entry_find(NVS_DEFAULT_PART_NAME, name_1, NVS_TYPE_ANY, &it) == ESP_OK);
-        nvs_entry_info(it, &info);
+        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);
+        it = nullptr;
-        while(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(nvs_entry_find(NVS_DEFAULT_PART_NAME, NAMESPACE, NVS_TYPE_I32, &it) == ESP_ERR_NVS_NOT_FOUND);
-    CHECK(it == NULL);
 
     // 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(nvs_entry_find(NVS_DEFAULT_PART_NAME, NAMESPACE, NVS_TYPE_STR, &it) == ESP_OK);
-    CHECK(it != NULL);
     nvs_release_iterator(it);
 
     // without deinit it affects "nvs api tests"

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_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` returns iterator to the next key-value pair.
+- :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

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_partition_name>, <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);
+    };
+
+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_partition_name>, <namespace>, 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.

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_find``：创建一个不透明句柄，用于后续调用 ``nvs_entry_next`` 和 ``nvs_entry_info`` 函数；
-- ``nvs_entry_next``：返回指向下一个键值对的迭代器；
+- ``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 句柄类。
 
 内部实现
 ---------

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);
+    nvs_iterator_t it = NULL;
-    if (it == NULL) {
+    esp_err_t result = nvs_entry_find(part, NULL, type, &it);
-        ESP_LOGE(TAG, "No such enty was found");
+    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;
 }

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