Skip to content

Commit 8986393

Browse files
Address review comments.
1 parent 8426bbb commit 8986393

File tree

1 file changed

+10
-4
lines changed

1 file changed

+10
-4
lines changed

src/app/util/attribute-table.cpp

+10-4
Original file line numberDiff line numberDiff line change
@@ -263,8 +263,8 @@ Status AttributeValueIsChanging(EndpointId endpoint, ClusterId cluster, Attribut
263263
// We don't know how to size our buffer for strings in general, but if the
264264
// string happens to fit into our fixed-size buffer, great.
265265
size_t valueSize = metadata->size;
266-
constexpr size_t maxValueSize = 16; // ipv6adr
267-
if (valueSize > maxValueSize)
266+
constexpr size_t kMaxValueSize = 16; // ipv6adr
267+
if (valueSize > kMaxValueSize)
268268
{
269269
if (emberAfIsStringAttributeType(attributeType) || emberAfIsLongStringAttributeType(attributeType))
270270
{
@@ -279,8 +279,8 @@ Status AttributeValueIsChanging(EndpointId endpoint, ClusterId cluster, Attribut
279279
return Status::ConstraintError;
280280
}
281281

282-
uint8_t oldValueBuffer[maxValueSize];
283-
// Cast to uint16_t is safe, because we checked valueSize <= maxValueSize above.
282+
uint8_t oldValueBuffer[kMaxValueSize];
283+
// Cast to uint16_t is safe, because we checked valueSize <= kMaxValueSize above.
284284
if (emberAfReadAttribute(endpoint, cluster, attributeID, oldValueBuffer, static_cast<uint16_t>(valueSize)) != Status::Success)
285285
{
286286
// We failed to read the old value, so flag the value as changing to be safe.
@@ -292,12 +292,18 @@ Status AttributeValueIsChanging(EndpointId endpoint, ClusterId cluster, Attribut
292292
{
293293
size_t oldLength = emberAfStringLength(oldValueBuffer);
294294
size_t newLength = emberAfStringLength(newValueData);
295+
// The first byte of the buffer is the string length, so comparing
296+
// oldLength to newLength handles comparing that byte, and the oldLength
297+
// bytes of actual data start one byte into the buffer.
295298
*isChanging = (oldLength != newLength) || (memcmp(oldValueBuffer + 1, newValueData + 1, oldLength) != 0);
296299
}
297300
else if (emberAfIsLongStringAttributeType(attributeType))
298301
{
299302
size_t oldLength = emberAfLongStringLength(oldValueBuffer);
300303
size_t newLength = emberAfLongStringLength(newValueData);
304+
// The first two bytes of the buffer are the string length, so comparing
305+
// oldLength to newLength handles comparing those bytes, and the oldLength
306+
// bytes of actual data start two bytes into the buffer.
301307
*isChanging = (oldLength != newLength) || (memcmp(oldValueBuffer + 2, newValueData + 2, oldLength) != 0);
302308
}
303309
else

0 commit comments

Comments
 (0)