TDBStore safety check: Erase if there is valid keys on the free space.

In case our are contains data from previous reset() or reset_area(),
we might  end up in the situation  where  free space contains valid
key headers,  but we have not erased that area  yet. This can cause
failures if the deinit() and init()  because  new scan of that area
would continue  as long as keys  are found. This causes keys on the
not-yet-erased  area to be included in the new instance of TDBStore.

To prevent this failure,  check  after each key-write that our free
space does not contain valid  key headers.  Also make sure  that we
erase one program unit sector over the master record.  If we erased
just the master record,first key might is still there, causing next
init() to find it.  Extend erase area by  one program unit, so that
build_ram_table() won't find any keys.

Author: Seppo Takalo

UNITTESTS/moduletests/storage/kvstore/TDBStore/moduletest.cpp

@@ -72,7 +72,7 @@ TEST_F(TDBStoreModuleTest, reset)
     size_t size, read;
     // Write so much, that we are sure that garbage collection have kicked up
     for (int i = 0; i < 100; ++i) {
-        size = sprintf(buf, "data%d", i);
+        size = sprintf(buf, "reset_%d", i);
         EXPECT_EQ(tdb.set("key", buf, size, 0), MBED_SUCCESS);
     }
     EXPECT_EQ(tdb.reset(), MBED_SUCCESS);
@@ -82,6 +82,17 @@ TEST_F(TDBStoreModuleTest, reset)
     EXPECT_NE(tdb.get("key", buf, 100, &read), MBED_SUCCESS);
 }
 
+TEST_F(TDBStoreModuleTest, remove)
+{
+    char buf[100];
+    size_t size;
+    EXPECT_EQ(tdb.set("key", "data1", 5, 0), MBED_SUCCESS);
+    EXPECT_EQ(tdb.set("key", "data2", 5, 0), MBED_SUCCESS);
+    EXPECT_EQ(tdb.remove("key"), MBED_SUCCESS);
+    // Previous key should not be found
+    EXPECT_NE(tdb.get("key", buf, 100, &size), MBED_SUCCESS);
+}
+
 TEST_F(TDBStoreModuleTest, set_deinit_init_get)
 {
     char buf[100];

features/storage/kvstore/tdbstore/TDBStore.cpp

@@ -144,6 +144,10 @@ TDBStore::~TDBStore()
 
 int TDBStore::read_area(uint8_t area, uint32_t offset, uint32_t size, void *buf)
 {
+    //Check that we are not crossing area boundary
+    if (offset + size > _size) {
+        return MBED_ERROR_READ_FAILED;
+    }
     int os_ret = _buff_bd->read(buf, _area_params[area].address + offset, size);
 
     if (os_ret) {
@@ -645,6 +649,15 @@ int TDBStore::set_finalize(set_handle_t handle)
 
     _free_space_offset = align_up(ih->bd_curr_offset, _prog_size);
 
+    // Safety check: If there seems to be valid keys on the free space
+    // we should erase one sector more, just to ensure that in case of power failure
+    // next init() would not extend the scan phase to that section as well.
+    os_ret = read_record(_active_area, _free_space_offset, 0, 0, 0, actual_data_size, 0,
+                          false, false, false, false, hash, flags, next_offset);
+    if (os_ret == MBED_SUCCESS) {
+        check_erase_before_write(_active_area, _free_space_offset, sizeof(record_header_t));
+    }
+
 end:
     if ((need_gc) && (ih->bd_base_offset != _master_record_offset)) {
         garbage_collection();
@@ -941,6 +954,7 @@ int TDBStore::increment_max_keys(void **ram_table)
     // Reallocate ram table with new size
     ram_table_entry_t *old_ram_table = (ram_table_entry_t *) _ram_table;
     ram_table_entry_t *new_ram_table = new ram_table_entry_t[_max_keys + 1];
+    memset(new_ram_table, 0, sizeof(ram_table_entry_t)*(_max_keys + 1));
 
     // Copy old content to new table
     memcpy(new_ram_table, old_ram_table, sizeof(ram_table_entry_t) * _max_keys);
@@ -987,6 +1001,7 @@ int TDBStore::init()
     _max_keys = initial_max_keys;
 
     ram_table = new ram_table_entry_t[_max_keys];
+    memset(ram_table, 0, sizeof(ram_table_entry_t) * _max_keys);
     _ram_table = ram_table;
     _num_keys = 0;
 
@@ -1127,7 +1142,7 @@ int TDBStore::reset_area(uint8_t area)
     bool copy_reserved_data = do_reserved_data_get(buf, sizeof(buf), 0, buf + RESERVED_AREA_SIZE) == MBED_SUCCESS;
 
     // Erase reserved area and master record
-    ret = check_erase_before_write(area, 0, _master_record_offset + _master_record_size, true);
+    ret = check_erase_before_write(area, 0, _master_record_offset + _master_record_size + _prog_size, true);
     if (ret) {
         return ret;
     }
@@ -1150,7 +1165,7 @@ int TDBStore::reset()
 
     // Reset both areas
     for (area = 0; area < _num_areas; area++) {
-        ret = check_erase_before_write(area, 0, _master_record_offset + _master_record_size, true);
+        ret = check_erase_before_write(area, 0, _master_record_offset + _master_record_size + _prog_size, true);
         if (ret) {
             goto end;
         }
@@ -1160,7 +1175,7 @@ int TDBStore::reset()
     _num_keys = 0;
     _free_space_offset = _master_record_offset;
     _active_area_version = 1;
-
+    memset(_ram_table, 0, sizeof(ram_table_entry_t) * _max_keys);
     // Write an initial master record on active area
     ret = write_master_record(_active_area, _active_area_version, _free_space_offset);
 

features/storage/kvstore/tdbstore/TDBStore.h

@@ -368,8 +368,8 @@ class TDBStore : public KVStore {
      *
      * @param[in]  area                   Area.
      * @param[in]  offset                 Offset of record in area.
-     * @param[in]  key                    Key - must not include '*' '/' '?' ':' ';' '\' '"' '|' ' ' '<' '>' '\'.
-     * @param[in]  data_buf               Data buffer.
+     * @param[out] key                    Key - must not include '*' '/' '?' ':' ';' '\' '"' '|' ' ' '<' '>' '\'.
+     * @param[out] data_buf               Data buffer.
      * @param[in]  data_buf_size          Data buffer size.
      * @param[out] actual_data_size       Actual data size.
      * @param[in]  data_offset            Offset in data.