Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change persistent storage SyncGetKeyValue API to match constraints of KvsStorageManager #20164

Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 8 additions & 9 deletions examples/chip-tool/config/PersistentStorage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,8 @@ CHIP_ERROR PersistentStorage::SyncGetKeyValue(const char * key, void * value, ui
{
std::string iniValue;

ReturnErrorCodeIf(((value == nullptr) && (size != 0)), CHIP_ERROR_INVALID_ARGUMENT);

auto section = mConfig.sections[kDefaultSectionName];
auto it = section.find(key);
ReturnErrorCodeIf(it == section.end(), CHIP_ERROR_PERSISTED_STORAGE_VALUE_NOT_FOUND);
Expand All @@ -111,17 +113,14 @@ CHIP_ERROR PersistentStorage::SyncGetKeyValue(const char * key, void * value, ui

iniValue = Base64ToString(iniValue);

uint16_t dataSize = static_cast<uint16_t>(iniValue.size());
if (dataSize > size)
{
size = dataSize;
return CHIP_ERROR_BUFFER_TOO_SMALL;
}
ReturnErrorCodeIf(((value == nullptr) && (size == 0)), CHIP_ERROR_BUFFER_TOO_SMALL);
tehampson marked this conversation as resolved.
Show resolved Hide resolved

size = dataSize;
memcpy(value, iniValue.data(), dataSize);
uint16_t dataSize = static_cast<uint16_t>(iniValue.size());
uint16_t sizeToCopy = std::min(size, dataSize);

return CHIP_NO_ERROR;
memcpy(value, iniValue.data(), sizeToCopy);
size = sizeToCopy;
return size < dataSize ? CHIP_ERROR_BUFFER_TOO_SMALL : CHIP_NO_ERROR;
}

CHIP_ERROR PersistentStorage::SyncSetKeyValue(const char * key, const void * value, uint16_t size)
Expand Down
19 changes: 10 additions & 9 deletions src/controller/python/ChipDeviceController-StorageDelegate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,28 +31,26 @@ namespace Controller {

CHIP_ERROR PythonPersistentStorageDelegate::SyncGetKeyValue(const char * key, void * value, uint16_t & size)
{
auto val = mStorage.find(key);
if (val == mStorage.end())
if ((value == nullptr) && (size != 0))
{
return CHIP_ERROR_PERSISTED_STORAGE_VALUE_NOT_FOUND;
return CHIP_ERROR_INVALID_ARGUMENT;
}

if (value == nullptr)
auto val = mStorage.find(key);
if (val == mStorage.end())
{
size = 0;
return CHIP_ERROR_PERSISTED_STORAGE_VALUE_NOT_FOUND;
}

uint16_t neededSize = val->second.size();
if (size == 0)
if ((value == nullptr) && (size == 0))
tehampson marked this conversation as resolved.
Show resolved Hide resolved
{
size = neededSize;
return CHIP_ERROR_BUFFER_TOO_SMALL;
}

if (size < neededSize)
{
memcpy(value, val->second.data(), size);
size = neededSize;
return CHIP_ERROR_BUFFER_TOO_SMALL;
}

Expand Down Expand Up @@ -86,6 +84,10 @@ namespace Python {
CHIP_ERROR StorageAdapter::SyncGetKeyValue(const char * key, void * value, uint16_t & size)
{
ChipLogDetail(Controller, "StorageAdapter::GetKeyValue: Key = %s, Value = %p (%u)", key, value, size);
if ((value == nullptr) && (size != 0))
{
return CHIP_ERROR_INVALID_ARGUMENT;
}

uint16_t tmpSize = size;

Expand All @@ -99,7 +101,6 @@ CHIP_ERROR StorageAdapter::SyncGetKeyValue(const char * key, void * value, uint1
if (size < tmpSize)
tehampson marked this conversation as resolved.
Show resolved Hide resolved
{
ChipLogDetail(Controller, "Buf not big enough\n");
size = tmpSize;
return CHIP_ERROR_BUFFER_TOO_SMALL;
}

Expand Down
19 changes: 15 additions & 4 deletions src/controller/python/chip/storage/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,23 +46,34 @@ def _OnSyncSetKeyValueCb(storageObj, key: str, value, size):

@_SyncGetKeyValueCbFunct
def _OnSyncGetKeyValueCb(storageObj, key: str, value, size):
''' This does not adhere API requirement of
tehampson marked this conversation as resolved.
Show resolved Hide resolved
PersistentStorageDelegate::SyncGetKeyValue, but that is okay since
the C++ storage binding layer is capable of adapting results from
this method to the requirements of
PersistentStorageDelegate::SyncGetKeyValue.
'''
try:
keyValue = storageObj.GetSdkKey(key.decode("utf-8"))
except Exception as ex:
keyValue = None

if (keyValue):
if (size[0] < len(keyValue)):
size[0] = len(keyValue)
return
sizeOfValue = size[0]
sizeToCopy = min(sizeOfValue, len(keyValue))

count = 0

for idx, val in enumerate(keyValue):
if sizeToCopy == count:
break
value[idx] = val
count = count + 1

size[0] = count
# As mentioned above, we are intentionally not returning
# sizeToCopy as one might expected because the caller
tehampson marked this conversation as resolved.
Show resolved Hide resolved
# will use the value in size[0] to determine if it should
# return CHIP_ERROR_BUFFER_TOO_SMALL.
size[0] = len(keyValue)
tehampson marked this conversation as resolved.
Show resolved Hide resolved
else:
size[0] = 0

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,19 +45,17 @@
}

if ([value length] > UINT16_MAX) {
error = CHIP_ERROR_BUFFER_TOO_SMALL;
size = 0;
error = CHIP_ERROR_PERSISTED_STORAGE_FAILED;
return;
}

uint16_t valueSize = static_cast<uint16_t>([value length]);
if (valueSize > size) {
error = CHIP_ERROR_BUFFER_TOO_SMALL;
} else {
size = valueSize;
return;
}

size = valueSize;
if (size != 0) {
// buffer is known to be non-null here.
memcpy(buffer, [value bytes], size);
Expand Down
41 changes: 27 additions & 14 deletions src/lib/core/CHIPPersistentStorageDelegate.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,30 +46,26 @@ class DLL_EXPORT PersistentStorageDelegate
* Caller is responsible to take care of any special formatting needs (e.g. byte
* order, null terminators, consistency checks or versioning).
*
* This API allows for determining the size of a stored value. Whenever
* the passed `size` is smaller than needed and the key exists in storage, the error
* CHIP_ERROR_BUFFER_TOO_SMALL will be given, and the `size` will be updated to the
* size of the stored value. It is legal to use `nullptr` for `buffer` if `size` is 0.
*
* If a key is found and the `buffer`'s `size` is large enough, then the value will
* be copied to `buffer` and `size` will be updated to the actual size used.
*
* The easiest way to determine if a key exists (and the value's size if so) is to pass
* `size` of 0, which is always valid to do, and will return CHIP_ERROR_BUFFER_TOO_SMALL
* if the key exists and CHIP_ERROR_PERSISTED_STORAGE_VALUE_NOT_FOUND if the
* key is not found.
* Whenever the passed `size` is smaller than the value for which the key exists in storage,
tehampson marked this conversation as resolved.
Show resolved Hide resolved
* CHIP_ERROR_BUFFER_TOO_SMALL will be given, but the `buffer` will still be filled `size`
* bytes of the stored value.
tehampson marked this conversation as resolved.
Show resolved Hide resolved
*
* A way to determine if the key exists is to pass `size` of 0, which is always valid to do,
* and check if CHIP_ERROR_BUFFER_TOO_SMALL is returned. Alternatively, the helper
tehampson marked this conversation as resolved.
Show resolved Hide resolved
* method SyncDoesKeyExist(key) can be used to more easily achieve the same effect.
*
* It is legal to use `nullptr` for `buffer` if `size` is 0.
*
* @param[in] key Key to lookup
* @param[out] buffer Pointer to a buffer where the place the read value.
* @param[in, out] size Input is maximum buffer size, output updated to length of value.
tehampson marked this conversation as resolved.
Show resolved Hide resolved
*
* @return CHIP_ERROR_PERSISTED_STORAGE_VALUE_NOT_FOUND the key is not found in storage.
* @return CHIP_ERROR_BUFFER_TOO_SMALL the provided buffer is not big enough. In this case
* "size" will indicate the needed buffer size. Some data
* may or may not be placed in "buffer" in this case; consumers
* should not rely on that behavior. CHIP_ERROR_BUFFER_TOO_SMALL
* combined with setting "size" to 0 means the actual size was
* too large to fit in uint16_t.
* `buffer` will be filled up to `size`.
tehampson marked this conversation as resolved.
Show resolved Hide resolved
*/
virtual CHIP_ERROR SyncGetKeyValue(const char * key, void * buffer, uint16_t & size) = 0;

Expand Down Expand Up @@ -97,6 +93,23 @@ class DLL_EXPORT PersistentStorageDelegate
* or another CHIP_ERROR value from implementation on failure.
*/
virtual CHIP_ERROR SyncDeleteKeyValue(const char * key) = 0;

/**
* @brief
* Helper function that identifies if a key exists.
tehampson marked this conversation as resolved.
Show resolved Hide resolved
*
* This may be overridden to provide an implementation that is simpler or more direct.
*
* @param[in] key Key to check if it exist
*
* @return true if key exists in storage. It returns false if key does not exist in storage or an internal error arises.
*/
virtual bool SyncDoesKeyExist(const char * key)
{
uint16_t size = 0;
CHIP_ERROR err = SyncGetKeyValue(key, nullptr, size);
return (err == CHIP_ERROR_BUFFER_TOO_SMALL) || (err == CHIP_NO_ERROR);
}
};

} // namespace chip
Loading