Skip to content

Commit 6f340c5

Browse files
lazarkovlazarkov
authored
Merge branch 'master' into feature/app-install-flow-public
2 parents 3f949ef + 05e4c10 commit 6f340c5

4 files changed

+45
-35
lines changed

‎src/app/DefaultAttributePersistenceProvider.cpp

+16-11
Original file line numberDiff line numberDiff line change
@@ -37,34 +37,40 @@ CHIP_ERROR DefaultAttributePersistenceProvider::InternalWriteValue(const Storage
3737
return mStorage->SyncSetKeyValue(aKey.KeyName(), aValue.data(), static_cast<uint16_t>(aValue.size()));
3838
}
3939

40-
CHIP_ERROR DefaultAttributePersistenceProvider::InternalReadValue(const StorageKeyName & aKey, EmberAfAttributeType aType,
41-
size_t aSize, MutableByteSpan & aValue)
40+
CHIP_ERROR DefaultAttributePersistenceProvider::InternalReadValue(const StorageKeyName & aKey, MutableByteSpan & aValue)
4241
{
4342
VerifyOrReturnError(mStorage != nullptr, CHIP_ERROR_INCORRECT_STATE);
4443

4544
uint16_t size = static_cast<uint16_t>(min(aValue.size(), static_cast<size_t>(UINT16_MAX)));
4645
ReturnErrorOnFailure(mStorage->SyncGetKeyValue(aKey.KeyName(), aValue.data(), size));
47-
EmberAfAttributeType type = aType;
48-
if (emberAfIsStringAttributeType(type))
46+
aValue.reduce_size(size);
47+
return CHIP_NO_ERROR;
48+
}
49+
50+
CHIP_ERROR DefaultAttributePersistenceProvider::InternalReadValue(const StorageKeyName & aKey, EmberAfAttributeType aType,
51+
size_t aExpectedSize, MutableByteSpan & aValue)
52+
{
53+
ReturnErrorOnFailure(InternalReadValue(aKey, aValue));
54+
size_t size = aValue.size();
55+
if (emberAfIsStringAttributeType(aType))
4956
{
5057
// Ensure that we've read enough bytes that we are not ending up with
5158
// un-initialized memory. Should have read length + 1 (for the length
5259
// byte).
53-
VerifyOrReturnError(size >= emberAfStringLength(aValue.data()) + 1, CHIP_ERROR_INCORRECT_STATE);
60+
VerifyOrReturnError(size >= 1 && size - 1 >= emberAfStringLength(aValue.data()), CHIP_ERROR_INCORRECT_STATE);
5461
}
55-
else if (emberAfIsLongStringAttributeType(type))
62+
else if (emberAfIsLongStringAttributeType(aType))
5663
{
5764
// Ensure that we've read enough bytes that we are not ending up with
5865
// un-initialized memory. Should have read length + 2 (for the length
5966
// bytes).
60-
VerifyOrReturnError(size >= emberAfLongStringLength(aValue.data()) + 2, CHIP_ERROR_INCORRECT_STATE);
67+
VerifyOrReturnError(size >= 2 && size - 2 >= emberAfLongStringLength(aValue.data()), CHIP_ERROR_INCORRECT_STATE);
6168
}
6269
else
6370
{
6471
// Ensure we got the expected number of bytes for all other types.
65-
VerifyOrReturnError(size == aSize, CHIP_ERROR_INVALID_ARGUMENT);
72+
VerifyOrReturnError(size == aExpectedSize, CHIP_ERROR_INVALID_ARGUMENT);
6673
}
67-
aValue.reduce_size(size);
6874
return CHIP_NO_ERROR;
6975
}
7076

@@ -90,8 +96,7 @@ CHIP_ERROR DefaultAttributePersistenceProvider::SafeWriteValue(const ConcreteAtt
9096
CHIP_ERROR DefaultAttributePersistenceProvider::SafeReadValue(const ConcreteAttributePath & aPath, MutableByteSpan & aValue)
9197
{
9298
return InternalReadValue(
93-
DefaultStorageKeyAllocator::SafeAttributeValue(aPath.mEndpointId, aPath.mClusterId, aPath.mAttributeId), 0x20,
94-
aValue.size(), aValue);
99+
DefaultStorageKeyAllocator::SafeAttributeValue(aPath.mEndpointId, aPath.mClusterId, aPath.mAttributeId), aValue);
95100
}
96101

97102
namespace {

‎src/app/DefaultAttributePersistenceProvider.h

+3-1
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,9 @@ class DefaultAttributePersistenceProvider : public AttributePersistenceProvider,
6363

6464
private:
6565
CHIP_ERROR InternalWriteValue(const StorageKeyName & aKey, const ByteSpan & aValue);
66-
CHIP_ERROR InternalReadValue(const StorageKeyName & aKey, EmberAfAttributeType aType, size_t aSize, MutableByteSpan & aValue);
66+
CHIP_ERROR InternalReadValue(const StorageKeyName & aKey, MutableByteSpan & aValue);
67+
CHIP_ERROR InternalReadValue(const StorageKeyName & aKey, EmberAfAttributeType aType, size_t aExpectedSize,
68+
MutableByteSpan & aValue);
6769
};
6870

6971
} // namespace app

‎src/app/SafeAttributePersistenceProvider.h

+18-22
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,8 @@ class SafeAttributePersistenceProvider
4040

4141
// The following API provides helper functions to simplify the access of commonly used types.
4242
// The API may not be complete.
43-
// Currently implemented write and read types are: uint8_t, uint16_t, uint32_t, unit64_t and
44-
// their nullable varieties, and bool.
43+
// Currently implemented write and read types are: bool, uint8_t, uint16_t, uint32_t, unit64_t and
44+
// their nullable varieties, as well as ByteSpans.
4545

4646
/**
4747
* Write an attribute value of type intX, uintX or bool to non-volatile memory.
@@ -50,7 +50,7 @@ class SafeAttributePersistenceProvider
5050
* @param [in] aValue the data to write.
5151
*/
5252
template <typename T, std::enable_if_t<std::is_integral<T>::value, bool> = true>
53-
CHIP_ERROR WriteScalarValue(const ConcreteAttributePath & aPath, T & aValue)
53+
CHIP_ERROR WriteScalarValue(const ConcreteAttributePath & aPath, T aValue)
5454
{
5555
uint8_t value[sizeof(T)];
5656
auto w = Encoding::LittleEndian::BufferWriter(value, sizeof(T));
@@ -71,18 +71,16 @@ class SafeAttributePersistenceProvider
7171
*
7272
* @param [in] aPath the attribute path for the data being persisted.
7373
* @param [in,out] aValue where to place the data.
74+
*
75+
* @retval CHIP_ERROR_PERSISTED_STORAGE_VALUE_NOT_FOUND if no stored value exists for the attribute
7476
*/
7577
template <typename T, std::enable_if_t<std::is_integral<T>::value, bool> = true>
7678
CHIP_ERROR ReadScalarValue(const ConcreteAttributePath & aPath, T & aValue)
7779
{
7880
uint8_t attrData[sizeof(T)];
7981
MutableByteSpan tempVal(attrData);
80-
auto err = SafeReadValue(aPath, tempVal);
81-
if (err != CHIP_NO_ERROR)
82-
{
83-
return err;
84-
}
85-
82+
ReturnErrorOnFailure(SafeReadValue(aPath, tempVal));
83+
VerifyOrReturnError(tempVal.size() == sizeof(T), CHIP_ERROR_INCORRECT_STATE);
8684
Encoding::LittleEndian::Reader r(tempVal.data(), tempVal.size());
8785
r.RawReadLowLevelBeCareful(&aValue);
8886
return r.StatusCode();
@@ -95,7 +93,7 @@ class SafeAttributePersistenceProvider
9593
* @param [in] aValue the data to write.
9694
*/
9795
template <typename T, std::enable_if_t<std::is_integral<T>::value, bool> = true>
98-
CHIP_ERROR WriteScalarValue(const ConcreteAttributePath & aPath, DataModel::Nullable<T> & aValue)
96+
CHIP_ERROR WriteScalarValue(const ConcreteAttributePath & aPath, const DataModel::Nullable<T> & aValue)
9997
{
10098
typename NumericAttributeTraits<T>::StorageType storageValue;
10199
if (aValue.IsNull())
@@ -114,16 +112,14 @@ class SafeAttributePersistenceProvider
114112
*
115113
* @param [in] aPath the attribute path for the data being persisted.
116114
* @param [in,out] aValue where to place the data.
115+
*
116+
* @retval CHIP_ERROR_PERSISTED_STORAGE_VALUE_NOT_FOUND if no stored value exists for the attribute
117117
*/
118118
template <typename T, std::enable_if_t<std::is_integral<T>::value, bool> = true>
119119
CHIP_ERROR ReadScalarValue(const ConcreteAttributePath & aPath, DataModel::Nullable<T> & aValue)
120120
{
121121
typename NumericAttributeTraits<T>::StorageType storageValue;
122-
CHIP_ERROR err = ReadScalarValue(aPath, storageValue);
123-
if (err != CHIP_NO_ERROR)
124-
{
125-
return err;
126-
}
122+
ReturnErrorOnFailure(ReadScalarValue(aPath, storageValue));
127123

128124
if (NumericAttributeTraits<T>::IsNullValue(storageValue))
129125
{
@@ -137,25 +133,25 @@ class SafeAttributePersistenceProvider
137133
return CHIP_NO_ERROR;
138134
}
139135

140-
protected:
141136
/**
142137
* Write an attribute value from the attribute store (i.e. not a struct or
143138
* list) to non-volatile memory.
144139
*
145140
* @param [in] aPath the attribute path for the data being written.
146-
* @param [in] aValue the data to write. The value should be stored as-is.
141+
* @param [in] aValue the data to write. The value will be stored as-is.
147142
*/
148143
virtual CHIP_ERROR SafeWriteValue(const ConcreteAttributePath & aPath, const ByteSpan & aValue) = 0;
149144

150145
/**
151-
* Read an attribute value from non-volatile memory.
152-
* It can be assumed that this method will never be called upon to read
153-
* an attribute of type string or long-string.
146+
* Read an attribute value as a raw sequence of bytes from non-volatile memory.
154147
*
155148
* @param [in] aPath the attribute path for the data being persisted.
156149
* @param [in,out] aValue where to place the data. The size of the buffer
157-
* will be equal to `aValue.size()`. The callee is expected to adjust
158-
* aValue's size to the actual number of bytes read.
150+
* will be equal to `aValue.size()`. On success aValue.size()
151+
* will be the actual number of bytes read.
152+
*
153+
* @retval CHIP_ERROR_PERSISTED_STORAGE_VALUE_NOT_FOUND if no stored value exists for the attribute
154+
* @retval CHIP_ERROR_BUFFER_TOO_SMALL aValue.size() is too small to hold the value.
159155
*/
160156
virtual CHIP_ERROR SafeReadValue(const ConcreteAttributePath & aPath, MutableByteSpan & aValue) = 0;
161157
};

‎src/app/tests/TestAttributePersistenceProvider.cpp

+8-1
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,13 @@ TEST_F(TestAttributePersistenceProvider, TestStorageAndRetrivalByteSpans)
5959
MutableByteSpan valueReadBack(getArray);
6060
err = persistenceProvider.SafeReadValue(TestConcretePath, valueReadBack);
6161
EXPECT_EQ(err, CHIP_NO_ERROR);
62-
EXPECT_TRUE(std::equal(valueReadBack.begin(), valueReadBack.end(), value.begin(), value.end()));
62+
EXPECT_TRUE(valueReadBack.data_equal(value));
63+
64+
uint8_t getArrayThatIsLongerThanNeeded[10];
65+
MutableByteSpan valueReadBack2(getArrayThatIsLongerThanNeeded);
66+
err = persistenceProvider.SafeReadValue(TestConcretePath, valueReadBack2);
67+
EXPECT_EQ(err, CHIP_NO_ERROR);
68+
EXPECT_TRUE(valueReadBack2.data_equal(value));
6369

6470
// Finishing
6571
persistenceProvider.Shutdown();
@@ -320,6 +326,7 @@ TEST_F(TestAttributePersistenceProvider, TestBufferTooSmallErrors)
320326
err = persistenceProvider.SafeReadValue(TestConcretePath, valueReadBackByteSpan8);
321327
EXPECT_EQ(err, CHIP_ERROR_BUFFER_TOO_SMALL);
322328

329+
// TODO: ReadScalarValue() does not take a buffer, so expecting CHIP_ERROR_BUFFER_TOO_SMALL is bad API
323330
// Fail to get value as uint8_t
324331
uint8_t valueReadBack8;
325332
err = persistenceProvider.ReadScalarValue(TestConcretePath, valueReadBack8);

0 commit comments

Comments
 (0)