Merge pull request #13996 from LDong-Arm/tdb_buf_size

TDBStore: pad program units when writing record_header_t; ensure work buffer is large enough

Author: 0xc0170

storage/kvstore/include/kvstore/TDBStore.h

@@ -308,6 +308,7 @@ class TDBStore : public KVStore {
     tdbstore_area_data_t _area_params[_num_areas];
     uint32_t _prog_size;
     uint8_t *_work_buf;
+    size_t _work_buf_size;
     char *_key_buf;
     void *_inc_set_handle;
     void *_iterator_table[_max_open_iterators];

storage/kvstore/source/TDBStore.cpp

@@ -77,7 +77,7 @@ typedef struct {
     uint32_t crc;
 } reserved_trailer_t;
 
-static const uint32_t work_buf_size = 64;
+static const size_t min_work_buf_size = 64;
 static const uint32_t initial_crc = 0xFFFFFFFF;
 static const uint32_t initial_max_keys = 16;
 
@@ -130,7 +130,7 @@ static uint32_t calc_crc(uint32_t init_crc, uint32_t data_size, const void *data
 TDBStore::TDBStore(BlockDevice *bd) : _ram_table(0), _max_keys(0),
     _num_keys(0), _bd(bd), _buff_bd(0),  _free_space_offset(0), _master_record_offset(0),
     _master_record_size(0), _is_initialized(false), _active_area(0), _active_area_version(0), _size(0),
-    _area_params{}, _prog_size(0), _work_buf(0), _key_buf(0), _inc_set_handle(0)
+    _area_params{}, _prog_size(0), _work_buf(0), _work_buf_size(0), _key_buf(0), _inc_set_handle(0)
 {
     for (int i = 0; i < _num_areas; i++) {
         _area_params[i] = { 0 };
@@ -196,7 +196,7 @@ int TDBStore::erase_erase_unit(uint8_t area, uint32_t offset)
 void TDBStore::calc_area_params()
 {
     // TDBStore can't exceed 32 bits
-    bd_size_t bd_size = std::min(_bd->size(), (bd_size_t) 0x80000000L);
+    bd_size_t bd_size = std::min<bd_size_t>(_bd->size(), 0x80000000L);
 
     memset(_area_params, 0, sizeof(_area_params));
     size_t area_0_size = 0;
@@ -293,7 +293,7 @@ int TDBStore::read_record(uint8_t area, uint32_t offset, char *key,
         return MBED_ERROR_INVALID_SIZE;
     }
 
-    actual_data_size = std::min((size_t)data_buf_size, (size_t)data_size - data_offset);
+    actual_data_size = std::min<size_t>(data_buf_size, data_size - data_offset);
 
     if (copy_data && actual_data_size && !data_buf) {
         return MBED_ERROR_INVALID_ARGUMENT;
@@ -327,7 +327,7 @@ int TDBStore::read_record(uint8_t area, uint32_t offset, char *key,
                 user_key_ptr[key_size] = '\0';
             } else {
                 dest_buf = _work_buf;
-                chunk_size = std::min(key_size, work_buf_size);
+                chunk_size = std::min<size_t>(key_size, _work_buf_size);
             }
         } else {
             // This means that we're on the data part
@@ -337,13 +337,13 @@ int TDBStore::read_record(uint8_t area, uint32_t offset, char *key,
             // 3. After actual part is finished - read to work buffer
             // 4. Copy data flag not set - read to work buffer
             if (curr_data_offset < data_offset) {
-                chunk_size = std::min((size_t)work_buf_size, (size_t)(data_offset - curr_data_offset));
+                chunk_size = std::min<size_t>(_work_buf_size, (data_offset - curr_data_offset));
                 dest_buf = _work_buf;
             } else if (copy_data && (curr_data_offset < data_offset + actual_data_size)) {
                 chunk_size = actual_data_size;
                 dest_buf = static_cast<uint8_t *>(data_buf);
             } else {
-                chunk_size = std::min(work_buf_size, total_size);
+                chunk_size = std::min<size_t>(_work_buf_size, total_size);
                 dest_buf = _work_buf;
             }
         }
@@ -847,7 +847,10 @@ int TDBStore::copy_record(uint8_t from_area, uint32_t from_offset, uint32_t to_o
     }
 
     chunk_size = align_up(sizeof(record_header_t), _prog_size);
-    ret = write_area(1 - from_area, to_offset, chunk_size, &header);
+    // The record header takes up whole program units
+    memset(_work_buf, 0, chunk_size);
+    memcpy(_work_buf, &header, sizeof(record_header_t));
+    ret = write_area(1 - from_area, to_offset, chunk_size, _work_buf);
     if (ret) {
         return ret;
     }
@@ -857,7 +860,7 @@ int TDBStore::copy_record(uint8_t from_area, uint32_t from_offset, uint32_t to_o
     total_size -= chunk_size;
 
     while (total_size) {
-        chunk_size = std::min(total_size, work_buf_size);
+        chunk_size = std::min<size_t>(total_size, _work_buf_size);
         ret = read_area(from_area, from_offset, chunk_size, _work_buf);
         if (ret) {
             return ret;
@@ -1041,7 +1044,8 @@ int TDBStore::init()
     }
 
     _prog_size = _bd->get_program_size();
-    _work_buf = new uint8_t[work_buf_size];
+    _work_buf_size = std::max<size_t>(_prog_size, min_work_buf_size);
+    _work_buf = new uint8_t[_work_buf_size];
     _key_buf = new char[MAX_KEY_SIZE];
     _inc_set_handle = new inc_set_handle_t;
     memset(_inc_set_handle, 0, sizeof(inc_set_handle_t));

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

@@ -20,14 +20,14 @@
 #include "kvstore/TDBStore.h"
 #include <stdlib.h>
 
-#define BLOCK_SIZE (512)
+#define BLOCK_SIZE (256)
 #define DEVICE_SIZE (BLOCK_SIZE*200)
 
 using namespace mbed;
 
 class TDBStoreModuleTest : public testing::Test {
 protected:
-    HeapBlockDevice heap{DEVICE_SIZE};
+    HeapBlockDevice heap{DEVICE_SIZE, BLOCK_SIZE};
     FlashSimBlockDevice flash{&heap};
     TDBStore tdb{&flash};
 
@@ -77,15 +77,17 @@ TEST_F(TDBStoreModuleTest, erased_set_get)
 
 TEST_F(TDBStoreModuleTest, set_deinit_init_get)
 {
-    char buf[100];
     size_t size;
     for (int i = 0; i < 100; ++i) {
-        EXPECT_EQ(tdb.set("key", "data", 5, 0), MBED_SUCCESS);
+        char str[11] = {0};
+        char buf[100] = {0};
+        int len = snprintf(str, 11, "data%d", i) + 1;
+        EXPECT_EQ(tdb.set("key", str, len, 0), MBED_SUCCESS);
         EXPECT_EQ(tdb.deinit(), MBED_SUCCESS);
         EXPECT_EQ(tdb.init(), MBED_SUCCESS);
         EXPECT_EQ(tdb.get("key", buf, 100, &size), MBED_SUCCESS);
-        EXPECT_EQ(size, 5);
-        EXPECT_STREQ("data", buf);
+        EXPECT_EQ(size, len);
+        EXPECT_STREQ(str, buf);
         EXPECT_EQ(tdb.remove("key"), MBED_SUCCESS);
     }
 }