From f32b25ebe242bd88f54f84732c745bce5d283fd0 Mon Sep 17 00:00:00 2001 From: Ivan Grokhotkov Date: Mon, 18 Sep 2017 22:30:21 +0800 Subject: [PATCH 1/4] nvs_flash: add functions to deinitialize storage --- components/nvs_flash/include/nvs_flash.h | 29 +++++++++++++++++-- components/nvs_flash/src/nvs_api.cpp | 36 ++++++++++++++++++++++++ components/nvs_flash/test/test_nvs.c | 4 +++ 3 files changed, 66 insertions(+), 3 deletions(-) diff --git a/components/nvs_flash/include/nvs_flash.h b/components/nvs_flash/include/nvs_flash.h index c9e4a72d74..a7ef7f4511 100644 --- a/components/nvs_flash/include/nvs_flash.h +++ b/components/nvs_flash/include/nvs_flash.h @@ -24,7 +24,7 @@ extern "C" { * @brief Initialize the default NVS partition. * * This API initialises the default NVS partition. The default NVS partition - * is the one that is labelled "nvs" in the partition table. + * is the one that is labeled "nvs" in the partition table. * * @return * - ESP_OK if storage was successfully initialized. @@ -38,7 +38,7 @@ esp_err_t nvs_flash_init(void); /** * @brief Initialize NVS flash storage for the specified partition. * - * @param[in] partition_name Name (label) of the partition. Note that internally a reference to + * @param[in] partition_label Label of the partition. Note that internally a reference to * passed value is kept and it should be accessible for future operations * * @return @@ -48,7 +48,30 @@ esp_err_t nvs_flash_init(void); * - ESP_ERR_NOT_FOUND if specified partition is not found in the partition table * - one of the error codes from the underlying flash storage driver */ -esp_err_t nvs_flash_init_partition(const char *partition_name); +esp_err_t nvs_flash_init_partition(const char *partition_label); + +/** + * @brief Deinitialize NVS storage for the default NVS partition + * + * Default NVS partition is the partition with "nvs" label in the partition table. + * + * @return + * - ESP_OK on success (storage was deinitialized) + * - ESP_ERR_NVS_NOT_INITIALIZED if the storage was not initialized prior to this call + */ +esp_err_t nvs_flash_deinit(void); + +/** + * @brief Deinitialize NVS storage for the given NVS partition + * + * @param[in] partition_label Label of the partition + * + * @return + * - ESP_OK on success + * - ESP_ERR_NVS_NOT_INITIALIZED if the storage for given partition was not + * initialized prior to this call + */ +esp_err_t nvs_flash_deinit_partition(const char* partition_label); /** * @brief Erase the default NVS partition diff --git a/components/nvs_flash/src/nvs_api.cpp b/components/nvs_flash/src/nvs_api.cpp index e5c2f5adcd..dba8e115a0 100644 --- a/components/nvs_flash/src/nvs_api.cpp +++ b/components/nvs_flash/src/nvs_api.cpp @@ -126,6 +126,42 @@ extern "C" esp_err_t nvs_flash_init(void) return nvs_flash_init_partition(NVS_DEFAULT_PART_NAME); } +extern "C" esp_err_t nvs_flash_deinit_partition(const char* partition_name) +{ + Lock::init(); + Lock lock; + + nvs::Storage* storage = lookup_storage_from_name(partition_name); + if (!storage) { + return ESP_ERR_NVS_NOT_INITIALIZED; + } + + /* Clean up handles related to the storage being deinitialized */ + auto it = s_nvs_handles.begin(); + auto next = it; + while(it != s_nvs_handles.end()) { + next++; + if (it->mStoragePtr == storage) { + ESP_LOGD(TAG, "Deleting handle %d (ns=%d) related to partition \"%s\" (missing call to nvs_close?)", + it->mHandle, it->mNsIndex, partition_name); + s_nvs_handles.erase(it); + delete static_cast(it); + } + it = next; + } + + /* Finally delete the storage itself */ + s_nvs_storage_list.erase(storage); + delete storage; + + return ESP_OK; +} + +extern "C" esp_err_t nvs_flash_deinit(void) +{ + return nvs_flash_deinit_partition(NVS_DEFAULT_PART_NAME); +} + extern "C" esp_err_t nvs_flash_erase_partition(const char *part_name) { const esp_partition_t* partition = esp_partition_find_first( diff --git a/components/nvs_flash/test/test_nvs.c b/components/nvs_flash/test/test_nvs.c index 07d01db468..ed0884a7d8 100644 --- a/components/nvs_flash/test/test_nvs.c +++ b/components/nvs_flash/test/test_nvs.c @@ -59,5 +59,9 @@ TEST_CASE("various nvs tests", "[nvs]") TEST_ASSERT_EQUAL_INT32(0, strcmp(buf, str)); nvs_close(handle_1); + + // check that deinit does not leak memory if some handles are still open + nvs_flash_deinit(); + nvs_close(handle_2); } From fe307891496bd511413eeba62353a4e04adfe866 Mon Sep 17 00:00:00 2001 From: Ivan Grokhotkov Date: Mon, 18 Sep 2017 22:34:16 +0800 Subject: [PATCH 2/4] nvs_flash: delete Storage if init fails Previously, nvs_flash_init_custom would not be called if Storage for a particular partition was already created. This caused issues if the first call to nvs_flash_init failed due to Storage init error. This issue exhibited itself as random failures of NVS CI test. With this change, storage object is deleted (and not added to storage list) if initialization fails. --- components/nvs_flash/src/nvs_api.cpp | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/components/nvs_flash/src/nvs_api.cpp b/components/nvs_flash/src/nvs_api.cpp index dba8e115a0..3502c6ca10 100644 --- a/components/nvs_flash/src/nvs_api.cpp +++ b/components/nvs_flash/src/nvs_api.cpp @@ -88,15 +88,22 @@ extern "C" void nvs_dump(const char *partName) extern "C" esp_err_t nvs_flash_init_custom(const char *partName, uint32_t baseSector, uint32_t sectorCount) { ESP_LOGD(TAG, "nvs_flash_init_custom partition=%s start=%d count=%d", partName, baseSector, sectorCount); - nvs::Storage* mStorage; - - mStorage = lookup_storage_from_name(partName); - if (mStorage == NULL) { - mStorage = new nvs::Storage((const char *)partName); - s_nvs_storage_list.push_back(mStorage); + nvs::Storage* new_storage = NULL; + nvs::Storage* storage = lookup_storage_from_name(partName); + if (storage == NULL) { + new_storage = new nvs::Storage((const char *)partName); + storage = new_storage; } - return mStorage->init(baseSector, sectorCount); + esp_err_t err = storage->init(baseSector, sectorCount); + if (new_storage != NULL) { + if (err == ESP_OK) { + s_nvs_storage_list.push_back(new_storage); + } else { + delete new_storage; + } + } + return err; } #ifdef ESP_PLATFORM From 01c0c4b661dee401ecda55633876afe5336f9612 Mon Sep 17 00:00:00 2001 From: Ivan Grokhotkov Date: Mon, 18 Sep 2017 22:41:58 +0800 Subject: [PATCH 3/4] unit-test-app: initialize partition table info before starting the test --- tools/unit-test-app/components/unity/unity_platform.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tools/unit-test-app/components/unity/unity_platform.c b/tools/unit-test-app/components/unity/unity_platform.c index ef985f2711..8e73fd5257 100644 --- a/tools/unit-test-app/components/unity/unity_platform.c +++ b/tools/unit-test-app/components/unity/unity_platform.c @@ -10,6 +10,7 @@ #include "esp_log.h" #include "soc/cpu.h" #include "esp_heap_caps.h" +#include "test_utils.h" #define unity_printf ets_printf @@ -37,6 +38,7 @@ const size_t CRITICAL_LEAK_THRESHOLD = 4096; void setUp(void) { printf("%s", ""); /* sneakily lazy-allocate the reent structure for this test task */ + get_test_data_partition(); /* allocate persistent partition table structures */ before_free_8bit = heap_caps_get_free_size(MALLOC_CAP_8BIT); before_free_32bit = heap_caps_get_free_size(MALLOC_CAP_32BIT); From 9325f2a7a4cd8ce7a6b82f074b7b389561c51d1b Mon Sep 17 00:00:00 2001 From: Ivan Grokhotkov Date: Mon, 18 Sep 2017 22:43:03 +0800 Subject: [PATCH 4/4] nvs_flash: emulator: fix issues in load function, add save function --- .../nvs_flash/test_nvs_host/spi_flash_emulation.h | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/components/nvs_flash/test_nvs_host/spi_flash_emulation.h b/components/nvs_flash/test_nvs_host/spi_flash_emulation.h index 14e56bab6e..d8099322d5 100644 --- a/components/nvs_flash/test_nvs_host/spi_flash_emulation.h +++ b/components/nvs_flash/test_nvs_host/spi_flash_emulation.h @@ -148,10 +148,20 @@ public: fseek(f, 0, SEEK_END); off_t size = ftell(f); assert(size % SPI_FLASH_SEC_SIZE == 0); - mData.resize(size); + mData.resize(size / sizeof(uint32_t)); fseek(f, 0, SEEK_SET); auto s = fread(mData.data(), SPI_FLASH_SEC_SIZE, size / SPI_FLASH_SEC_SIZE, f); assert(s == static_cast(size / SPI_FLASH_SEC_SIZE)); + fclose(f); + } + + void save(const char* filename) + { + FILE* f = fopen(filename, "wb"); + auto n_sectors = mData.size() * sizeof(uint32_t) / SPI_FLASH_SEC_SIZE; + auto s = fwrite(mData.data(), SPI_FLASH_SEC_SIZE, n_sectors, f); + assert(s == n_sectors); + fclose(f); } void clearStats()