Skip to content

Commit

Permalink
Update KVS to handle zero length elements (project-chip#21977)
Browse files Browse the repository at this point in the history 
* allow for nullptr in KVS write
* complete audit with chip_support_enable_storage_api_audit=true
  • Loading branch information
@srickardti @isiu-apple
srickardti authored and isiu-apple committed Sep 16, 2022
1 parent b6df04d commit 741fd8c
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 40 deletions.
83 changes: 49 additions & 34 deletions src/platform/cc13x2_26x2/CC13X2_26X2Config.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
#include <platform/internal/testing/ConfigUnitTest.h>

#include <lib/core/CHIPEncoding.h>
#include <lib/core/CHIPPersistentStorageDelegate.h>
#include <lib/support/CodeUtils.h>
#include <platform/cc13x2_26x2/CC13X2_26X2Config.h>

Expand Down Expand Up @@ -165,7 +166,7 @@ CHIP_ERROR CC13X2_26X2Config::ReadConfigValueBin(Key key, uint8_t * buf, size_t
/* Iterate through the key range to find a key that matches. */
static uint8_t FindKVSSubID(const char * key, uint16_t & subID)
{
char key_scratch[32]; // 32 characters seems large enough for a key
char key_scratch[PersistentStorageDelegate::kKeyLengthMax + 1];
NVINTF_nvProxy_t nvProxy = { 0 };
uint8_t status = NVINTF_SUCCESS;

Expand Down Expand Up @@ -209,20 +210,23 @@ CHIP_ERROR CC13X2_26X2Config::ReadKVS(const char * key, void * value, size_t val
val_item.subID = subID;

len = sNvoctpFps.getItemLen(val_item);
VerifyOrExit(len > 0, err = CHIP_DEVICE_ERROR_CONFIG_NOT_FOUND); // key not found

if ((offset_bytes + value_size) > len)
if (value_size >= (len - offset_bytes))
{
// trying to read up to the end of the element
// reading to end of element
read_len = len - offset_bytes;
}
else
{
read_len = value_size;
err = CHIP_ERROR_BUFFER_TOO_SMALL;
}

VerifyOrExit(sNvoctpFps.readItem(val_item, (uint16_t) offset_bytes, read_len, value) == NVINTF_SUCCESS,
err = CHIP_ERROR_PERSISTED_STORAGE_FAILED);
if (read_len > 0)
{
VerifyOrExit(sNvoctpFps.readItem(val_item, (uint16_t) offset_bytes, read_len, value) == NVINTF_SUCCESS,
err = CHIP_ERROR_PERSISTED_STORAGE_FAILED);
}

if (read_bytes_size)
{
Expand Down Expand Up @@ -264,22 +268,14 @@ CHIP_ERROR CC13X2_26X2Config::WriteKVS(const char * key, const void * value, siz
CHIP_ERROR err = CHIP_NO_ERROR;
uint16_t subID;

if (FindKVSSubID(key, subID) == NVINTF_SUCCESS)
{
NVINTF_itemID_t val_item = CC13X2_26X2Config::kConfigKey_KVS_value.nvID;
// key already exists, update value
val_item.subID = subID;
VerifyOrExit(sNvoctpFps.updateItem(val_item, (uint16_t) value_size, (void *) value) == NVINTF_SUCCESS,
err = CHIP_ERROR_PERSISTED_STORAGE_FAILED);
}
else
NVINTF_itemID_t key_item = CC13X2_26X2Config::kConfigKey_KVS_key.nvID;
NVINTF_itemID_t val_item = CC13X2_26X2Config::kConfigKey_KVS_value.nvID;

if (FindKVSSubID(key, subID) != NVINTF_SUCCESS)
{
// key does not exist, likely case
// key item not found, find an empty subID
intptr_t lock_key = sNvoctpFps.lockNV();

NVINTF_itemID_t key_item = CC13X2_26X2Config::kConfigKey_KVS_key.nvID;
NVINTF_itemID_t val_item = CC13X2_26X2Config::kConfigKey_KVS_value.nvID;

/* Iterate through the subID range to find an unused subID in the
* keyspace. SubID is a 10 bit value, reference
* `<simplelink_sdk>/source/ti/common/nv/nvocmp.c:MVOCMP_MAXSUBID`.
Expand All @@ -289,26 +285,39 @@ CHIP_ERROR CC13X2_26X2Config::WriteKVS(const char * key, const void * value, siz
key_item.subID = i;
if (sNvoctpFps.getItemLen(key_item) == 0U)
{
val_item.subID = i;
subID = i;
break;
}
}
sNvoctpFps.unlockNV(lock_key);

// write they key item
if (sNvoctpFps.writeItem(key_item, (uint16_t) strlen(key), (void *) key) == NVINTF_SUCCESS)
VerifyOrExit(sNvoctpFps.writeItem(key_item, (uint16_t) strlen(key), (void *) key) == NVINTF_SUCCESS,
err = CHIP_ERROR_PERSISTED_STORAGE_FAILED);
}

key_item.subID = subID;
val_item.subID = subID;

if (value_size == 0U)
{
// delete the value item if it exists
int8_t ret = sNvoctpFps.deleteItem(val_item);
if (ret != NVINTF_SUCCESS && ret != NVINTF_NOTFOUND)
{
if (sNvoctpFps.writeItem(val_item, (uint16_t) value_size, (void *) value) != NVINTF_SUCCESS)
{
// try to delete the key item
sNvoctpFps.deleteItem(key_item);
err = CHIP_ERROR_PERSISTED_STORAGE_FAILED;
}
err = CHIP_ERROR_PERSISTED_STORAGE_FAILED;
}
else
}
else
{
if (sNvoctpFps.writeItem(val_item, (uint16_t) value_size, (void *) value) != NVINTF_SUCCESS)
{
// try to delete the key item
sNvoctpFps.deleteItem(key_item);
err = CHIP_ERROR_PERSISTED_STORAGE_FAILED;
}
sNvoctpFps.unlockNV(lock_key);
}

exit:
return err;
}
Expand All @@ -325,25 +334,31 @@ CHIP_ERROR CC13X2_26X2Config::WriteConfigValueBin(Key key, const uint8_t * data,

CHIP_ERROR CC13X2_26X2Config::ClearKVS(const char * key)
{
CHIP_ERROR err = CHIP_NO_ERROR;
CHIP_ERROR err = CHIP_ERROR_PERSISTED_STORAGE_VALUE_NOT_FOUND;
uint16_t subID;
NVINTF_itemID_t key_item = CC13X2_26X2Config::kConfigKey_KVS_key.nvID;
NVINTF_itemID_t val_item = CC13X2_26X2Config::kConfigKey_KVS_value.nvID;

if (FindKVSSubID(key, subID) == NVINTF_SUCCESS)
{
int8_t ret;

key_item.subID = subID;
val_item.subID = subID;
// delete the value item
if (sNvoctpFps.deleteItem(val_item) != NVINTF_SUCCESS)
// delete the value item if it exists
ret = sNvoctpFps.deleteItem(val_item);
if (ret != NVINTF_SUCCESS && ret != NVINTF_NOTFOUND)
{
err = CHIP_ERROR_PERSISTED_STORAGE_FAILED;
}
// delete the key item
if (sNvoctpFps.deleteItem(key_item) != NVINTF_SUCCESS)
// delete the key item if it exists
ret = sNvoctpFps.deleteItem(key_item);
if (ret != NVINTF_SUCCESS && ret != NVINTF_NOTFOUND)
{
err = CHIP_ERROR_PERSISTED_STORAGE_FAILED;
}

err = CHIP_NO_ERROR;
}

return err;
Expand Down
7 changes: 4 additions & 3 deletions src/platform/cc13x2_26x2/KeyValueStoreManagerImpl.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,10 @@ CHIP_ERROR KeyValueStoreManagerImpl::_Get(const char * key, void * value, size_t
CHIP_ERROR err;

VerifyOrReturnError(key, CHIP_ERROR_INVALID_ARGUMENT);
VerifyOrReturnError(value, CHIP_ERROR_INVALID_ARGUMENT);
if (0U != value_size)
{
VerifyOrReturnError(value, CHIP_ERROR_INVALID_ARGUMENT);
}

err = CC13X2_26X2Config::ReadKVS(key, value, value_size, read_bytes_size, offset_bytes);

Expand All @@ -58,8 +61,6 @@ CHIP_ERROR KeyValueStoreManagerImpl::_Get(const char * key, void * value, size_t
CHIP_ERROR KeyValueStoreManagerImpl::_Put(const char * key, const void * value, size_t value_size)
{
VerifyOrReturnError(key, CHIP_ERROR_INVALID_ARGUMENT);
VerifyOrReturnError(value, CHIP_ERROR_INVALID_ARGUMENT);
VerifyOrReturnError(value_size > 0, CHIP_ERROR_INVALID_ARGUMENT);

return CC13X2_26X2Config::WriteKVS(key, value, value_size);
}
Expand Down
7 changes: 4 additions & 3 deletions src/platform/cc32xx/KeyValueStoreManagerImpl.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,16 +42,17 @@ CHIP_ERROR KeyValueStoreManagerImpl::_Get(const char * key, void * value, size_t
size_t offset_bytes) const
{
VerifyOrReturnError(key, CHIP_ERROR_INVALID_ARGUMENT);
VerifyOrReturnError(value, CHIP_ERROR_INVALID_ARGUMENT);
if (0U != value_size)
{
VerifyOrReturnError(value, CHIP_ERROR_INVALID_ARGUMENT);
}

return CC32XXConfig::ReadKVS(key, value, value_size, read_bytes_size, offset_bytes);
}

CHIP_ERROR KeyValueStoreManagerImpl::_Put(const char * key, const void * value, size_t value_size)
{
VerifyOrReturnError(key, CHIP_ERROR_INVALID_ARGUMENT);
VerifyOrReturnError(value, CHIP_ERROR_INVALID_ARGUMENT);
VerifyOrReturnError(value_size > 0, CHIP_ERROR_INVALID_ARGUMENT);

return CC32XXConfig::WriteKVS(key, value, value_size);
}
Expand Down

0 comments on commit 741fd8c

Please sign in to comment.