diff --git a/features/storage/TESTS/kvstore/general_tests_phase_1/main.cpp b/features/storage/TESTS/kvstore/general_tests_phase_1/main.cpp index 2dab435e7b..3e0a4abb24 100644 --- a/features/storage/TESTS/kvstore/general_tests_phase_1/main.cpp +++ b/features/storage/TESTS/kvstore/general_tests_phase_1/main.cpp @@ -258,9 +258,7 @@ static void set_several_keys_multithreaded() for (i = 0; i < num_of_threads; i++) { res = kvstore->get(keys[i], buffer, buffer_size, &actual_size, 0); TEST_ASSERT_EQUAL_ERROR_CODE(MBED_SUCCESS, res); - TEST_ASSERT_EQUAL_STRING_LEN(buffer, data, data_size); - } for (i = 0; i < num_of_threads; i++) { @@ -329,6 +327,7 @@ static void set_key_value_two_byte_size() TEST_ASSERT_EQUAL_ERROR_CODE(MBED_SUCCESS, res); res = kvstore->get(key, buffer, buffer_size, &actual_size, 0); + TEST_ASSERT_EQUAL_ERROR_CODE(MBED_SUCCESS, res); TEST_ASSERT_EQUAL_STRING_LEN(buffer, data_two, 1); memset(buffer, 0, buffer_size); @@ -346,6 +345,7 @@ static void set_key_value_five_byte_size() TEST_ASSERT_EQUAL_ERROR_CODE(MBED_SUCCESS, res); res = kvstore->get(key, buffer, buffer_size, &actual_size, 0); + TEST_ASSERT_EQUAL_ERROR_CODE(MBED_SUCCESS, res); TEST_ASSERT_EQUAL_STRING_LEN(buffer, data_five, 4); memset(buffer, 0, buffer_size); @@ -363,6 +363,7 @@ static void set_key_value_fifteen_byte_size() TEST_ASSERT_EQUAL_ERROR_CODE(MBED_SUCCESS, res); res = kvstore->get(key, buffer, buffer_size, &actual_size, 0); + TEST_ASSERT_EQUAL_ERROR_CODE(MBED_SUCCESS, res); TEST_ASSERT_EQUAL_STRING_LEN(buffer, data_fif, 14); memset(buffer, 0, buffer_size); @@ -380,6 +381,7 @@ static void set_key_value_seventeen_byte_size() TEST_ASSERT_EQUAL_ERROR_CODE(MBED_SUCCESS, res); res = kvstore->get(key, buffer, buffer_size, &actual_size, 0); + TEST_ASSERT_EQUAL_ERROR_CODE(MBED_SUCCESS, res); TEST_ASSERT_EQUAL_STRING_LEN(buffer, data_fif, 16); memset(buffer, 0, buffer_size); @@ -451,7 +453,8 @@ static void set_key_init_deinit() TEST_ASSERT_EQUAL_ERROR_CODE(MBED_SUCCESS, res); res = kvstore->get(key, buffer, buffer_size, &actual_size, 0); - TEST_ASSERT_EQUAL_STRING(buffer, data); + TEST_ASSERT_EQUAL_ERROR_CODE(MBED_SUCCESS, res); + TEST_ASSERT_EQUAL_STRING(data, buffer); memset(buffer, 0, buffer_size); res = kvstore->remove(key); diff --git a/features/storage/kvstore/tdbstore/TDBStore.cpp b/features/storage/kvstore/tdbstore/TDBStore.cpp index c12b15e09b..01034d139d 100644 --- a/features/storage/kvstore/tdbstore/TDBStore.cpp +++ b/features/storage/kvstore/tdbstore/TDBStore.cpp @@ -390,6 +390,7 @@ int TDBStore::set_start(set_handle_t *handle, const char *key, size_t final_data uint32_t offset; uint32_t hash, ram_table_ind; inc_set_handle_t *ih; + bool need_gc = false; if (!is_valid_key(key)) { return MBED_ERROR_INVALID_ARGUMENT; @@ -485,14 +486,19 @@ int TDBStore::set_start(set_handle_t *handle, const char *key, size_t final_data // Write key now ret = write_area(_active_area, ih->bd_curr_offset, ih->header.key_size, key); if (ret) { + need_gc = true; goto fail; } ih->bd_curr_offset += ih->header.key_size; goto end; fail: + if ((need_gc) && (ih->bd_base_offset != _master_record_offset)) { + garbage_collection(); + } // mark handle as invalid by clearing magic field in header ih->header.magic = 0; + _mutex.unlock(); end: @@ -503,6 +509,7 @@ int TDBStore::set_add_data(set_handle_t handle, const void *value_data, size_t d { int ret = MBED_SUCCESS; inc_set_handle_t *ih; + bool need_gc = false; if (handle != _inc_set_handle) { return MBED_ERROR_INVALID_ARGUMENT; @@ -532,12 +539,17 @@ int TDBStore::set_add_data(set_handle_t handle, const void *value_data, size_t d // Write the data chunk ret = write_area(_active_area, ih->bd_curr_offset, data_size, value_data); if (ret) { + need_gc = true; goto end; } ih->bd_curr_offset += data_size; ih->offset_in_data += data_size; end: + if ((need_gc) && (ih->bd_base_offset != _master_record_offset)) { + garbage_collection(); + } + _inc_set_mutex.unlock(); return ret; } @@ -548,6 +560,8 @@ int TDBStore::set_finalize(set_handle_t handle) inc_set_handle_t *ih; ram_table_entry_t *ram_table = (ram_table_entry_t *) _ram_table; ram_table_entry_t *entry; + bool need_gc = false; + uint32_t actual_data_size, hash, flags, next_offset; if (handle != _inc_set_handle) { return MBED_ERROR_INVALID_ARGUMENT; @@ -563,14 +577,14 @@ int TDBStore::set_finalize(set_handle_t handle) if (ih->offset_in_data != ih->header.data_size) { ret = MBED_ERROR_INVALID_SIZE; - // Need GC as otherwise our storage is left in a non-usable state - garbage_collection(); + need_gc = true; goto end; } // Write header ret = write_area(_active_area, ih->bd_base_offset, sizeof(record_header_t), &ih->header); if (ret) { + need_gc = true; goto end; } @@ -578,6 +592,7 @@ int TDBStore::set_finalize(set_handle_t handle) os_ret = _buff_bd->sync(); if (os_ret) { ret = MBED_ERROR_WRITE_FAILED; + need_gc = true; goto end; } @@ -586,6 +601,16 @@ int TDBStore::set_finalize(set_handle_t handle) goto end; } + // Writes may fail without returning a failure (specially in flash components). Reread the record + // to ensure write success (this won't read the data anywhere - just use the CRC calculation). + ret = read_record(_active_area, ih->bd_base_offset, 0, 0, (uint32_t) -1, + actual_data_size, 0, false, false, false, false, + hash, flags, next_offset); + if (ret) { + need_gc = true; + goto end; + } + // Update RAM table if (ih->header.flags & delete_flag) { _num_keys--; @@ -611,10 +636,15 @@ int TDBStore::set_finalize(set_handle_t handle) _free_space_offset = align_up(ih->bd_curr_offset, _prog_size); end: + if ((need_gc) && (ih->bd_base_offset != _master_record_offset)) { + garbage_collection(); + } + // mark handle as invalid by clearing magic field in header ih->header.magic = 0; _inc_set_mutex.unlock(); + if (ih->bd_base_offset != _master_record_offset) { _mutex.unlock(); }