From 2d0ee5519044454c1c76a4a07f31be066d4d5155 Mon Sep 17 00:00:00 2001 From: Keith Wheeler <42851569+keithmwheeler@users.noreply.github.com> Date: Thu, 25 Aug 2022 10:03:49 -0700 Subject: [PATCH] =?UTF-8?q?Updating=20Infineon=20PSoC6=20key=20value=20sto?= =?UTF-8?q?re=20manager=20to=20cover=20all=20requirem=E2=80=A6=20(#22029)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Updating Infineon PSoC6 key value store manager to cover all requirements of persistent storage audit * Correcting defect in handling zero length key values --- .../PSOC6/KeyValueStoreManagerImpl.cpp | 53 +++++++++++++++++-- 1 file changed, 50 insertions(+), 3 deletions(-) diff --git a/src/platform/Infineon/PSOC6/KeyValueStoreManagerImpl.cpp b/src/platform/Infineon/PSOC6/KeyValueStoreManagerImpl.cpp index 2cb94828e8cef1..40555fa42fea40 100644 --- a/src/platform/Infineon/PSOC6/KeyValueStoreManagerImpl.cpp +++ b/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(&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(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(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(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(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); }