NVStore: key management enhancements

- Define an enum for predefined keys (later filled by internal users of NVStore)
- Add the set_alloc_key API, allocating a free key from the non predefined keys

Author: David Saada

features/nvstore/README.md

@@ -22,6 +22,7 @@ Each item is kept in an entry containing a header and data, where the header hol
 - get: Get the value of an item, given key.
 - set: Set the value of an item, given key and value.
 - set_once: Like set, but allows only a one time setting of this item (and disables deleting of this item).
+- set_alloc_key: Like set, but allocates a free key (from the non predefined keys).
 - remove: Remove an item, given key.
 - get_item_size: Get the item value size (in bytes).
 - set_max_keys: Set maximal value of unique keys. Overriding the default of NVSTORE_MAX_KEYS. This affects RAM consumption,

features/nvstore/TESTS/nvstore/functionality/main.cpp

@@ -70,6 +70,7 @@ static void nvstore_basic_functionality_test()
 
     uint16_t actual_len_bytes = 0;
     NVStore &nvstore = NVStore::get_instance();
+    uint16_t key;
 
     uint8_t *nvstore_testing_buf_set = new uint8_t[basic_func_max_data_size];
     uint8_t *nvstore_testing_buf_get = new uint8_t[basic_func_max_data_size];
@@ -127,6 +128,10 @@ static void nvstore_basic_functionality_test()
     result = nvstore.set(19, 10, &(nvstore_testing_buf_set[3]));
     TEST_ASSERT_EQUAL(NVSTORE_ALREADY_EXISTS, result);
 
+    result = nvstore.set_alloc_key(key, 17, &(nvstore_testing_buf_set[3]));
+    TEST_ASSERT_EQUAL(NVSTORE_NUM_PREDEFINED_KEYS, key);
+    TEST_ASSERT_EQUAL(NVSTORE_SUCCESS, result);
+
     // Make sure set items are also gotten OK after reset
     result = nvstore.deinit();
     TEST_ASSERT_EQUAL(NVSTORE_SUCCESS, result);
@@ -153,6 +158,11 @@ static void nvstore_basic_functionality_test()
     TEST_ASSERT_EQUAL(64, actual_len_bytes);
     TEST_ASSERT_EQUAL_UINT8_ARRAY(nvstore_testing_buf_set, nvstore_testing_buf_get, 64);
 
+    result = nvstore.get(NVSTORE_NUM_PREDEFINED_KEYS, 64, nvstore_testing_buf_get, actual_len_bytes);
+    TEST_ASSERT_EQUAL(NVSTORE_SUCCESS, result);
+    TEST_ASSERT_EQUAL(17, actual_len_bytes);
+    TEST_ASSERT_EQUAL_UINT8_ARRAY(&nvstore_testing_buf_set[3], nvstore_testing_buf_get, 17);
+
     result = nvstore.get(10, 65, nvstore_testing_buf_get, actual_len_bytes);
     TEST_ASSERT_EQUAL(NVSTORE_SUCCESS, result);
     TEST_ASSERT_EQUAL(64, actual_len_bytes);

features/nvstore/source/nvstore.cpp

@@ -134,6 +134,7 @@ NVStore::NVStore() : _init_done(0), _init_attempts(0), _active_area(0), _max_key
       _active_area_version(0), _free_space_offset(0), _size(0), _mutex(0), _offset_by_key(0), _flash(0),
       _min_prog_size(0), _page_buf(0)
 {
+    memcpy(_flash_area_params, 0, sizeof(_flash_area_params));
 }
 
 NVStore::~NVStore()
@@ -589,6 +590,10 @@ int NVStore::do_get(uint16_t key, uint16_t buf_size, void *buf, uint16_t &actual
 
     if (!buf) {
         buf_size = 0;
+        // This is only required in order to satisfy static code analysis tools, fearing
+        // that a null buff is dereferenced inside read_record function. However, this won't happen
+        // when buf_size is 0, so just have buf point to a dummy location.
+        buf = &flags;
     }
 
     _mutex->lock();
@@ -623,7 +628,7 @@ int NVStore::get_item_size(uint16_t key, uint16_t &actual_size)
     return do_get(key, 0, NULL, actual_size, 1);
 }
 
-int NVStore::do_set(uint16_t key, uint16_t buf_size, const void *buf, uint16_t flags)
+int NVStore::do_set(uint16_t &key, uint16_t buf_size, const void *buf, uint16_t flags)
 {
     int ret = NVSTORE_SUCCESS;
     uint32_t record_offset, record_size, new_free_space;
@@ -636,7 +641,11 @@ int NVStore::do_set(uint16_t key, uint16_t buf_size, const void *buf, uint16_t f
         }
     }
 
-    if (key >= _max_keys) {
+    if ((key != no_key) && (key >= _max_keys)) {
+        return NVSTORE_BAD_VALUE;
+    }
+
+    if ((key == no_key) && (flags & delete_item_flag)) {
         return NVSTORE_BAD_VALUE;
     }
 
@@ -648,14 +657,25 @@ int NVStore::do_set(uint16_t key, uint16_t buf_size, const void *buf, uint16_t f
         return NVSTORE_NOT_FOUND;
     }
 
-    if (_offset_by_key[key] & offs_by_key_set_once_mask) {
+    if ((key != no_key) && (_offset_by_key[key] & offs_by_key_set_once_mask)) {
         return NVSTORE_ALREADY_EXISTS;
     }
 
     record_size = align_up(sizeof(nvstore_record_header_t) + buf_size, _min_prog_size);
 
     _mutex->lock();
 
+    if (key == no_key) {
+        for (key = NVSTORE_NUM_PREDEFINED_KEYS; key < _max_keys; key++) {
+            if (!_offset_by_key[key]) {
+                break;
+            }
+        }
+        if (key == _max_keys) {
+            return NVSTORE_NO_FREE_KEY;
+        }
+    }
+
     new_free_space = core_util_atomic_incr_u32(&_free_space_offset, record_size);
     record_offset = new_free_space - record_size;
 
@@ -696,6 +716,12 @@ int NVStore::set_once(uint16_t key, uint16_t buf_size, const void *buf)
     return do_set(key, buf_size, buf, set_once_flag);
 }
 
+int NVStore::set_alloc_key(uint16_t &key, uint16_t buf_size, const void *buf)
+{
+    key = no_key;
+    return do_set(key, buf_size, buf, 0);
+}
+
 int NVStore::remove(uint16_t key)
 {
     return do_set(key, 0, NULL, delete_item_flag);

features/nvstore/source/nvstore.h

@@ -41,10 +41,20 @@ typedef enum {
     NVSTORE_FLASH_AREA_TOO_SMALL   = -7,
     NVSTORE_OS_ERROR               = -8,
     NVSTORE_ALREADY_EXISTS         = -9,
+    NVSTORE_NO_FREE_KEY            = -10,
 } nvstore_status_e;
 
+typedef enum {
+    NVSTORE_FIRST_PREDEFINED_KEY        = 0,
+
+    // All predefined keys used for internal features should be defined here
+
+    NVSTORE_LAST_PREDEFINED_KEY         = 15,
+    NVSTORE_NUM_PREDEFINED_KEYS
+} nvstore_predefined_keys_e;
+
 #ifndef NVSTORE_MAX_KEYS
-#define NVSTORE_MAX_KEYS 16
+#define NVSTORE_MAX_KEYS ((uint16_t)NVSTORE_NUM_PREDEFINED_KEYS)
 #endif
 
 // defines 2 areas - active and nonactive, not configurable
@@ -148,6 +158,24 @@ class NVStore : private mbed::NonCopyable<NVStore> {
      */
     int set(uint16_t key, uint16_t buf_size, const void *buf);
 
+    /**
+     * @brief Programs one item of data on Flash, allocating a key.
+     *
+     * @param[out] key                  Returned key of stored item.
+     * @param[in]  buf_size             Item size in bytes.
+     * @param[in]  buf                  Buffer containing data.
+     *
+     * @returns NVSTORE_SUCCESS           Value was successfully written on Flash.
+     *          NVSTORE_WRITE_ERROR       Physical error writing data.
+     *          NVSTORE_BAD_VALUE         Bad value in any of the parameters.
+     *          NVSTORE_FLASH_AREA_TOO_SMALL
+     *                                    Not enough space in Flash area.
+     *          NVSTORE_ALREADY_EXISTS    Item set with write once API already exists.
+     *          NVSTORE_NO_FREE_KEY       Couldn't allocate a key for this call.
+     *
+     */
+    int set_alloc_key(uint16_t &key, uint16_t buf_size, const void *buf);
+
     /**
      * @brief Programs one item of data on Flash, given key, allowing no consequent sets to this key.
      *
@@ -394,14 +422,14 @@ class NVStore : private mbed::NonCopyable<NVStore> {
     /**
      * @brief Actual logics of set API (covers also set_once and remove APIs).
      *
-     * @param[in]  key                    key.
+     * @param[out] key                    key (both input and output).
      * @param[in]  buf_size               Buffer size (bytes).
      * @param[in]  buf                    Input Buffer.
      * @param[in]  flags                  Record flags.
      *
      * @returns 0 for success, nonzero for failure.
      */
-    int do_set(uint16_t key, uint16_t buf_size, const void *buf, uint16_t flags);
+    int do_set(uint16_t &key, uint16_t buf_size, const void *buf, uint16_t flags);
 
 };
 /** @}*/