Updating Infineon PSoC6 key value store manager to cover all requirem… (

project-chip#22029)

* Updating Infineon PSoC6 key value store manager to cover all requirements of persistent storage audit

* Correcting defect in handling zero length key values

src/platform/Infineon/PSOC6/KeyValueStoreManagerImpl.cpp

@@ -58,6 +58,9 @@ CHIP_ERROR KeyValueStoreManagerImpl::_Get(const char * key, void * value, size_t
     // Get the value size
     result = mtb_kvstore_read(const_cast<mtb_kvstore_t *>(&kvstore_obj), key, NULL, &actual_size);
 
+    // If fail, return the failure code to the caller.
+    // If success, but the value pointer is NULL, then the caller only wanted to know if the key
+    // exists and/or the size of the key's value. Set read_bytes_size (if non-NULL) and then return.
     if ((result != CY_RSLT_SUCCESS) || (value == NULL))
     {
         if (read_bytes_size != nullptr)
@@ -68,6 +71,19 @@ CHIP_ERROR KeyValueStoreManagerImpl::_Get(const char * key, void * value, size_t
         return ConvertCyResultToChip(result);
     }
 
+    // If actual size is zero, there is no value to read, case this function can return.
+    if (actual_size == 0)
+    {
+        if (read_bytes_size != nullptr)
+        {
+            *read_bytes_size = actual_size; // The calling matter api expects this to always be set
+        }
+        return CHIP_NO_ERROR;
+    }
+
+    // If the actual size of the stored key is larger than the buffer the caller provided, as indicated by value_size,
+    // then we need to copy as many bytes of the value as we can into the return buffer.
+    // Since the return buffer is too small, allocate storage big enough. Will be freed later in this function.
     if ((actual_size > value_size) || (offset_bytes != 0))
     {
         size = actual_size;
@@ -87,7 +103,7 @@ CHIP_ERROR KeyValueStoreManagerImpl::_Get(const char * key, void * value, size_t
 
         if (actual_size < value_size)
         {
-            // They may ask for more than what was originally stored, so we need to zero out the
+            // Caller may ask for more than what was originally stored, so we need to zero out the
             // entire value to account for that.
             memset(&((uint8_t *) value)[actual_size], 0, value_size - actual_size);
         }
@@ -101,6 +117,9 @@ CHIP_ERROR KeyValueStoreManagerImpl::_Get(const char * key, void * value, size_t
         return ConvertCyResultToChip(result);
     }
 
+    // If we allocated space for a value larger than the caller anticipated,
+    // then we need to copy as many bytes as we can into their provided buffer
+    // (e.g. value). After the copy, free our temporary buffer in local_value.
     if (local_value != value)
     {
         memcpy(value, &local_value[offset_bytes], value_size);
@@ -114,6 +133,8 @@ CHIP_ERROR KeyValueStoreManagerImpl::_Get(const char * key, void * value, size_t
             *read_bytes_size = static_cast<size_t>(value_size);
         }
 
+        // If the actual size of the value (minus any offset) is larger than the buffer
+        // provided to us, as defined by value_size, then we return the too small error code.
         if ((actual_size - offset_bytes) > value_size)
         {
             return CHIP_ERROR_BUFFER_TOO_SMALL;
@@ -134,7 +155,23 @@ CHIP_ERROR KeyValueStoreManagerImpl::_Put(const char * key, const void * value,
         return CHIP_ERROR_WELL_UNINITIALIZED;
     }
 
-    cy_rslt_t result = mtb_kvstore_write(&kvstore_obj, key, static_cast<const uint8_t *>(value), value_size);
+    cy_rslt_t result;
+
+    // This if statement is checking for a situation where the caller provided us a non-NULL value whose
+    // size is 0. Per the SyncSetKeyValue definition, it is valid to pass a non-NULL value with size 0.
+    // This will result in a key being written to storage, but with an empty value. However, the
+    // mtb-kvstore does not allow this. Instead, if you want to store a key with an empty value,
+    // mtb-kvstore requires you to pass NULL for value and 0 for size. So, this logic is translating
+    // between the two requirements.
+    if (value != NULL && static_cast<size_t>(value_size) == 0)
+    {
+        result = mtb_kvstore_write(&kvstore_obj, key, NULL, 0);
+    }
+    else
+    {
+        result = mtb_kvstore_write(&kvstore_obj, key, (uint8_t *) value, static_cast<size_t>(value_size));
+    }
+
     return ConvertCyResultToChip(result);
 }
 
@@ -145,7 +182,17 @@ CHIP_ERROR KeyValueStoreManagerImpl::_Delete(const char * key)
         return CHIP_ERROR_WELL_UNINITIALIZED;
     }
 
-    cy_rslt_t result = mtb_kvstore_delete(&kvstore_obj, key);
+    // Matter KVStore requires that a delete called on a key that doesn't exist return an error
+    // indicating no such key exists. mtb-kvstore returns success when asked to delete a key
+    // that doesn't exist. To translate between these two requirements, we use mtb_kvstore_read,
+    // which returns MTB_KVSTORE_ITEM_NOT_FOUND_ERROR if the key doesn't exist. We only call
+    // mtb_kvstore_delete if the key actually exists.
+    cy_rslt_t result = mtb_kvstore_read(&kvstore_obj, key, NULL, NULL);
+    if (result == CY_RSLT_SUCCESS)
+    {
+        result = mtb_kvstore_delete(&kvstore_obj, key);
+    }
+
     return ConvertCyResultToChip(result);
 }