diff --git a/components/nvs_flash/host_test/nvs_host_test/main/test_nvs.cpp b/components/nvs_flash/host_test/nvs_host_test/main/test_nvs.cpp index 8cdfae21a5..c9d174da7e 100644 --- a/components/nvs_flash/host_test/nvs_host_test/main/test_nvs.cpp +++ b/components/nvs_flash/host_test/nvs_host_test/main/test_nvs.cpp @@ -608,7 +608,7 @@ TEST_CASE("nvs api tests", "[nvs]") nvs_handle_t handle_2; TEST_ESP_OK(nvs_open("namespace2", NVS_READWRITE, &handle_2)); TEST_ESP_OK(nvs_set_i32(handle_2, "foo", 0x3456789a)); - const char *str = "value 0123456789abcdef0123456789abcdef"; + char str[] = "value 0123456789abcdef0123456789abcdef"; TEST_ESP_OK(nvs_set_str(handle_2, "key", str)); int32_t v1; @@ -619,7 +619,7 @@ TEST_CASE("nvs api tests", "[nvs]") TEST_ESP_OK(nvs_get_i32(handle_2, "foo", &v2)); CHECK(0x3456789a == v2); - char buf[strlen(str) + 1]; + char buf[sizeof(str)]; size_t buf_len = sizeof(buf); size_t buf_len_needed; @@ -1270,13 +1270,15 @@ public: } else { blobBufLen = largeBlobLen ; } - uint8_t buf[blobBufLen]; + uint8_t* buf = new uint8_t[blobBufLen]; memset(buf, 0, blobBufLen); size_t len = blobBufLen; auto err = nvs_get_blob(handle, keys[index], buf, &len); + auto eval_result = evaluate(delayCount, err, types[index], written[index], potentially_written[index], buf, values[index], future_values[index], blobBufLen); + delete [] buf; - REQUIRE(evaluate(delayCount, err, types[index], written[index], potentially_written[index], buf, values[index], future_values[index], blobBufLen) == true); + REQUIRE(eval_result == true); break; } @@ -1371,7 +1373,7 @@ public: } else { blobBufLen = largeBlobLen ; } - uint8_t buf[blobBufLen]; + uint8_t* buf = new uint8_t[blobBufLen]; memset(buf, 0, blobBufLen); size_t blobLen = gen() % blobBufLen; std::generate_n(buf, blobLen, [&]() -> uint8_t { @@ -1386,16 +1388,19 @@ public: if (err == ESP_ERR_FLASH_OP_FAIL) { // mark potentially written potentially_written[index] = true; + delete [] buf; return err; } if (err == ESP_ERR_NVS_REMOVE_FAILED) { written[index] = true; memcpy(reinterpret_cast(values[index]), buf, blobBufLen); + delete [] buf; return ESP_ERR_FLASH_OP_FAIL; } REQUIRE(err == ESP_OK); written[index] = true; memcpy(reinterpret_cast(values[index]), buf, blobBufLen); + delete [] buf; break; } @@ -2889,16 +2894,20 @@ TEST_CASE("inconsistent fields in item header with correct crc are handled for m // initial values uint32_t uval1 = 1; uint32_t uval2 = 2; - uint8_t blob_data1[blob_key1_chunk_0_len + blob_key1_chunk_1_len]; - uint8_t blob_data2[blob_key2_chunk_len]; + size_t blob_data1_len = blob_key1_chunk_0_len + blob_key1_chunk_1_len; + size_t blob_data2_len = blob_key2_chunk_len; + uint8_t* blob_data1 = new uint8_t[blob_data1_len]; + uint8_t* blob_data2 = new uint8_t[blob_data2_len]; // value buffers uint32_t read_u32_1; uint32_t read_u32_2; - uint8_t read_blob1[sizeof(blob_data1)]; - uint8_t read_blob2[sizeof(blob_data2)]; - size_t read_blob1_size = sizeof(read_blob1); - size_t read_blob2_size = sizeof(read_blob2); + + size_t read_blob1_size = blob_data1_len; + size_t read_blob2_size = blob_data2_len; + uint8_t* read_blob1 = new uint8_t[read_blob1_size]; + uint8_t* read_blob2 = new uint8_t[read_blob2_size]; + // Skip one page, 2 entries of page header and 3 entries of valueblob1's BLOB_DATA entries const size_t blob_index_offset = 4096 + 32 * 2 + 32 * 3; @@ -2921,10 +2930,10 @@ TEST_CASE("inconsistent fields in item header with correct crc are handled for m // initialize buffers read_u32_1 = 0; read_u32_2 = 0; - memset(blob_data1, 0x55, sizeof(blob_data1)); - memset(blob_data2, 0xaa, sizeof(blob_data2)); - memset(read_blob1, 0, sizeof(read_blob1)); - memset(read_blob2, 0, sizeof(read_blob2)); + memset(blob_data1, 0x55, blob_data1_len); + memset(blob_data2, 0xaa, blob_data2_len); + memset(read_blob1, 0, read_blob1_size); + memset(read_blob2, 0, read_blob2_size); // Write initial data to the nvs partition { @@ -2933,8 +2942,8 @@ TEST_CASE("inconsistent fields in item header with correct crc are handled for m nvs_handle_t handle; TEST_ESP_OK(nvs_open("test", NVS_READWRITE, &handle)); TEST_ESP_OK(nvs_set_u32(handle, ukey1, uval1)); - TEST_ESP_OK(nvs_set_blob(handle, blob_key1, blob_data1, sizeof(blob_data1))); - TEST_ESP_OK(nvs_set_blob(handle, blob_key2, blob_data2, sizeof(blob_data2))); + TEST_ESP_OK(nvs_set_blob(handle, blob_key1, blob_data1, blob_data1_len)); + TEST_ESP_OK(nvs_set_blob(handle, blob_key2, blob_data2, blob_data2_len)); TEST_ESP_OK(nvs_set_u32(handle, ukey2, uval2)); nvs_close(handle); @@ -3067,8 +3076,8 @@ TEST_CASE("inconsistent fields in item header with correct crc are handled for m nvs_handle_t handle; TEST_ESP_OK(nvs_open("test", NVS_READWRITE, &handle)); TEST_ESP_OK(nvs_get_u32(handle, ukey1, &read_u32_1)); - TEST_ESP_ERR(nvs_get_blob(handle, blob_key1, &read_blob1, &read_blob1_size), ESP_ERR_NVS_NOT_FOUND); - TEST_ESP_OK(nvs_get_blob(handle, blob_key2, &read_blob2, &read_blob2_size)); + TEST_ESP_ERR(nvs_get_blob(handle, blob_key1, read_blob1, &read_blob1_size), ESP_ERR_NVS_NOT_FOUND); + TEST_ESP_OK(nvs_get_blob(handle, blob_key2, read_blob2, &read_blob2_size)); TEST_ESP_OK(nvs_get_u32(handle, ukey2, &read_u32_2)); nvs_close(handle); TEST_ESP_OK(nvs_flash_deinit_partition(f.part()->get_partition_name())); @@ -3076,7 +3085,8 @@ TEST_CASE("inconsistent fields in item header with correct crc are handled for m // validate the data CHECK(read_u32_1 == uval1); CHECK(read_u32_2 == uval2); - CHECK(memcmp(read_blob2, blob_data2, sizeof(blob_data2)) == 0); + CHECK(read_blob2_size == blob_data2_len); + CHECK(memcmp(read_blob2, blob_data2, read_blob2_size) == 0); } } @@ -3808,6 +3818,8 @@ TEST_CASE("nvs multiple write with same key but different types", "[nvs]") TEST_ESP_OK(nvs_flash_deinit_partition(NVS_DEFAULT_PART_NAME)); } +#ifndef CONFIG_NVS_LEGACY_DUP_KEYS_COMPATIBILITY +// Following test case is not valid for new behavior not leading to multiple active values under the same key TEST_CASE("nvs multiple write with same key blob and string involved", "[nvs]") { PartitionEmulationFixture f(0, 10); @@ -3883,6 +3895,7 @@ TEST_CASE("nvs multiple write with same key blob and string involved", "[nvs]") TEST_ESP_OK(nvs_flash_deinit_partition(NVS_DEFAULT_PART_NAME)); } +#endif // !CONFIG_NVS_LEGACY_DUP_KEYS_COMPATIBILITY TEST_CASE("nvs find key tests", "[nvs]") { diff --git a/components/nvs_flash/host_test/nvs_host_test/pytest_nvs_host_linux.py b/components/nvs_flash/host_test/nvs_host_test/pytest_nvs_host_linux.py index 37b8275a7e..60172550f8 100644 --- a/components/nvs_flash/host_test/nvs_host_test/pytest_nvs_host_linux.py +++ b/components/nvs_flash/host_test/nvs_host_test/pytest_nvs_host_linux.py @@ -6,6 +6,14 @@ from pytest_embedded_idf.utils import idf_parametrize @pytest.mark.host_test +@pytest.mark.parametrize( + 'config', + [ + 'default_set_key', + 'legacy_set_key', + ], + indirect=True, +) @idf_parametrize('target', ['linux'], indirect=['target']) def test_nvs_host_linux(dut: Dut) -> None: dut.expect_exact('All tests passed', timeout=60) diff --git a/components/nvs_flash/src/nvs_storage.cpp b/components/nvs_flash/src/nvs_storage.cpp index 374e176757..f776404422 100644 --- a/components/nvs_flash/src/nvs_storage.cpp +++ b/components/nvs_flash/src/nvs_storage.cpp @@ -407,6 +407,27 @@ esp_err_t Storage::writeItem(uint8_t nsIndex, ItemType datatype, const char* key if(err == ESP_OK && findPage != nullptr) { matchedTypePageFound = true; } +#ifdef CONFIG_NVS_LEGACY_DUP_KEYS_COMPATIBILITY + // In legacy mode, we also try to find the item as BLOB if not found as BLOB_INDEX. + // In this mode, it is possible to have multiple active values under the same (logical) key. + // For BLOBs (which may have different physical representations in V1 it is BLOB, in V2 it is BLOB_INDEX) it in turn means + // that we have to check both datatypes to find the old value. + // The general case for compatibility flag disabled is below and handles all datatypes including BLOB. + // To save some cycles, we do not compile both findItem calls in this case. + if(err == ESP_ERR_NVS_NOT_FOUND) { + // If not found as BLOB_INDEX, try to find it as BLOB (legacy support). + err = findItem(nsIndex, ItemType::BLOB, key, findPage, item, Page::CHUNK_ANY, VerOffset::VER_ANY, &itemIndex); + if(err == ESP_OK && findPage != nullptr) { + matchedTypePageFound = false; // datatype does not match, we cannot extract chunkStart from the item + + // keep the sequence number of the page where the item was found for later check of relocation + err = findPage->getSeqNumber(findPageSeqNumber); + if(err != ESP_OK) { + return err; + } + } + } +#endif } else { // Handle all other data types than BLOB err = findItem(nsIndex, datatype, key, findPage, item, Page::CHUNK_ANY, VerOffset::VER_ANY, &itemIndex);