Skip to content

Commit

Permalink
address comments
Browse files Browse the repository at this point in the history
  • Loading branch information
yunhanw-google committed Jun 1, 2022
1 parent a403d87 commit 4d230f8
Show file tree
Hide file tree
Showing 40 changed files with 460 additions and 635 deletions.
25 changes: 9 additions & 16 deletions src/app/MessageDef/AttributeDataIB.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ AttributeDataIB::Parser::ParseData(TLV::TLVReader & aReader, int aDepth) const
CHIP_ERROR AttributeDataIB::Parser::CheckSchemaValidity() const
{
CHIP_ERROR err = CHIP_NO_ERROR;
int TagPresenceMask = 0;
int tagPresenceMask = 0;
TLV::TLVReader reader;

PRETTY_PRINT("AttributeDataIB =");
Expand All @@ -207,8 +207,8 @@ CHIP_ERROR AttributeDataIB::Parser::CheckSchemaValidity() const
{
case to_underlying(Tag::kDataVersion):
// check if this tag has appeared before
VerifyOrReturnError(!(TagPresenceMask & (1 << to_underlying(Tag::kDataVersion))), CHIP_ERROR_INVALID_TLV_TAG);
TagPresenceMask |= (1 << to_underlying(Tag::kDataVersion));
VerifyOrReturnError(!(tagPresenceMask & (1 << to_underlying(Tag::kDataVersion))), CHIP_ERROR_INVALID_TLV_TAG);
tagPresenceMask |= (1 << to_underlying(Tag::kDataVersion));
VerifyOrReturnError(TLV::kTLVType_UnsignedInteger == reader.GetType(), CHIP_ERROR_WRONG_TLV_TYPE);

#if CHIP_DETAIL_LOGGING
Expand All @@ -221,8 +221,8 @@ CHIP_ERROR AttributeDataIB::Parser::CheckSchemaValidity() const
break;
case to_underlying(Tag::kPath):
// check if this tag has appeared before
VerifyOrReturnError(!(TagPresenceMask & (1 << to_underlying(Tag::kPath))), CHIP_ERROR_INVALID_TLV_TAG);
TagPresenceMask |= (1 << to_underlying(Tag::kPath));
VerifyOrReturnError(!(tagPresenceMask & (1 << to_underlying(Tag::kPath))), CHIP_ERROR_INVALID_TLV_TAG);
tagPresenceMask |= (1 << to_underlying(Tag::kPath));
{
AttributePathIB::Parser path;
ReturnErrorOnFailure(path.Init(reader));
Expand All @@ -234,8 +234,8 @@ CHIP_ERROR AttributeDataIB::Parser::CheckSchemaValidity() const
break;
case to_underlying(Tag::kData):
// check if this tag has appeared before
VerifyOrReturnError(!(TagPresenceMask & (1 << to_underlying(Tag::kData))), CHIP_ERROR_INVALID_TLV_TAG);
TagPresenceMask |= (1 << to_underlying(Tag::kData));
VerifyOrReturnError(!(tagPresenceMask & (1 << to_underlying(Tag::kData))), CHIP_ERROR_INVALID_TLV_TAG);
tagPresenceMask |= (1 << to_underlying(Tag::kData));

PRETTY_PRINT_INCDEPTH();
ReturnErrorOnFailure(ParseData(reader, 0));
Expand All @@ -253,16 +253,9 @@ CHIP_ERROR AttributeDataIB::Parser::CheckSchemaValidity() const
if (CHIP_END_OF_TLV == err)
{
// check for required fields:
const int RequiredFields = (1 << to_underlying(Tag::kPath)) | (1 << to_underlying(Tag::kData));
const int requiredFields = (1 << to_underlying(Tag::kPath)) | (1 << to_underlying(Tag::kData));

if ((TagPresenceMask & RequiredFields) == RequiredFields)
{
err = CHIP_NO_ERROR;
}
else
{
err = CHIP_ERROR_IM_MALFORMED_ATTRIBUTE_DATA_ELEMENT;
}
err = (tagPresenceMask & requiredFields) == requiredFields ? CHIP_NO_ERROR : CHIP_ERROR_IM_MALFORMED_ATTRIBUTE_DATA_IB;
}
ReturnErrorOnFailure(err);
return reader.ExitContainer(mOuterContainerType);
Expand Down
34 changes: 17 additions & 17 deletions src/app/MessageDef/AttributePathIB.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ namespace app {
CHIP_ERROR AttributePathIB::Parser::CheckSchemaValidity() const
{
CHIP_ERROR err = CHIP_NO_ERROR;
int TagPresenceMask = 0;
int tagPresenceMask = 0;
TLV::TLVReader reader;

PRETTY_PRINT("AttributePathIB =");
Expand All @@ -53,8 +53,8 @@ CHIP_ERROR AttributePathIB::Parser::CheckSchemaValidity() const
{
case to_underlying(Tag::kEnableTagCompression):
// check if this tag has appeared before
VerifyOrReturnError(!(TagPresenceMask & (1 << to_underlying(Tag::kEnableTagCompression))), CHIP_ERROR_INVALID_TLV_TAG);
TagPresenceMask |= (1 << to_underlying(Tag::kEnableTagCompression));
VerifyOrReturnError(!(tagPresenceMask & (1 << to_underlying(Tag::kEnableTagCompression))), CHIP_ERROR_INVALID_TLV_TAG);
tagPresenceMask |= (1 << to_underlying(Tag::kEnableTagCompression));
#if CHIP_DETAIL_LOGGING
{
bool enableTagCompression;
Expand All @@ -66,8 +66,8 @@ CHIP_ERROR AttributePathIB::Parser::CheckSchemaValidity() const
case to_underlying(Tag::kNode):
// check if this tag has appeared before

VerifyOrReturnError(!(TagPresenceMask & (1 << to_underlying(Tag::kNode))), err = CHIP_ERROR_INVALID_TLV_TAG);
TagPresenceMask |= (1 << to_underlying(Tag::kNode));
VerifyOrReturnError(!(tagPresenceMask & (1 << to_underlying(Tag::kNode))), err = CHIP_ERROR_INVALID_TLV_TAG);
tagPresenceMask |= (1 << to_underlying(Tag::kNode));
VerifyOrReturnError(TLV::kTLVType_UnsignedInteger == reader.GetType(), err = CHIP_ERROR_WRONG_TLV_TYPE);

#if CHIP_DETAIL_LOGGING
Expand All @@ -80,8 +80,8 @@ CHIP_ERROR AttributePathIB::Parser::CheckSchemaValidity() const
break;
case to_underlying(Tag::kEndpoint):
// check if this tag has appeared before
VerifyOrReturnError(!(TagPresenceMask & (1 << to_underlying(Tag::kEndpoint))), CHIP_ERROR_INVALID_TLV_TAG);
TagPresenceMask |= (1 << to_underlying(Tag::kEndpoint));
VerifyOrReturnError(!(tagPresenceMask & (1 << to_underlying(Tag::kEndpoint))), CHIP_ERROR_INVALID_TLV_TAG);
tagPresenceMask |= (1 << to_underlying(Tag::kEndpoint));
VerifyOrReturnError(TLV::kTLVType_UnsignedInteger == reader.GetType(), CHIP_ERROR_WRONG_TLV_TYPE);
#if CHIP_DETAIL_LOGGING
{
Expand All @@ -93,8 +93,8 @@ CHIP_ERROR AttributePathIB::Parser::CheckSchemaValidity() const
break;
case to_underlying(Tag::kCluster):
// check if this tag has appeared before
VerifyOrReturnError(!(TagPresenceMask & (1 << to_underlying(Tag::kCluster))), err = CHIP_ERROR_INVALID_TLV_TAG);
TagPresenceMask |= (1 << to_underlying(Tag::kCluster));
VerifyOrReturnError(!(tagPresenceMask & (1 << to_underlying(Tag::kCluster))), err = CHIP_ERROR_INVALID_TLV_TAG);
tagPresenceMask |= (1 << to_underlying(Tag::kCluster));
VerifyOrReturnError(TLV::kTLVType_UnsignedInteger == reader.GetType(), err = CHIP_ERROR_WRONG_TLV_TYPE);

#if CHIP_DETAIL_LOGGING
Expand All @@ -107,8 +107,8 @@ CHIP_ERROR AttributePathIB::Parser::CheckSchemaValidity() const
break;
case to_underlying(Tag::kAttribute):
// check if this tag has appeared before
VerifyOrReturnError(!(TagPresenceMask & (1 << to_underlying(Tag::kAttribute))), CHIP_ERROR_INVALID_TLV_TAG);
TagPresenceMask |= (1 << to_underlying(Tag::kAttribute));
VerifyOrReturnError(!(tagPresenceMask & (1 << to_underlying(Tag::kAttribute))), CHIP_ERROR_INVALID_TLV_TAG);
tagPresenceMask |= (1 << to_underlying(Tag::kAttribute));
VerifyOrReturnError(TLV::kTLVType_UnsignedInteger == reader.GetType(), CHIP_ERROR_WRONG_TLV_TYPE);
#if CHIP_DETAIL_LOGGING
{
Expand All @@ -120,8 +120,8 @@ CHIP_ERROR AttributePathIB::Parser::CheckSchemaValidity() const
break;
case to_underlying(Tag::kListIndex):
// check if this tag has appeared before
VerifyOrReturnError(!(TagPresenceMask & (1 << to_underlying(Tag::kListIndex))), CHIP_ERROR_INVALID_TLV_TAG);
TagPresenceMask |= (1 << to_underlying(Tag::kListIndex));
VerifyOrReturnError(!(tagPresenceMask & (1 << to_underlying(Tag::kListIndex))), CHIP_ERROR_INVALID_TLV_TAG);
tagPresenceMask |= (1 << to_underlying(Tag::kListIndex));
VerifyOrReturnError(TLV::kTLVType_UnsignedInteger == reader.GetType() || TLV::kTLVType_Null == reader.GetType(),
CHIP_ERROR_WRONG_TLV_TYPE);
#if CHIP_DETAIL_LOGGING
Expand Down Expand Up @@ -149,10 +149,10 @@ CHIP_ERROR AttributePathIB::Parser::CheckSchemaValidity() const
// if we have exhausted this container
if (CHIP_END_OF_TLV == err)
{
if ((TagPresenceMask & (1 << to_underlying(Tag::kAttribute))) == 0 &&
(TagPresenceMask & (1 << to_underlying(Tag::kListIndex))) != 0)
if ((tagPresenceMask & (1 << to_underlying(Tag::kAttribute))) == 0 &&
(tagPresenceMask & (1 << to_underlying(Tag::kListIndex))) != 0)
{
err = CHIP_ERROR_IM_MALFORMED_ATTRIBUTE_PATH;
err = CHIP_ERROR_IM_MALFORMED_ATTRIBUTE_PATH_IB;
}
else
{
Expand Down Expand Up @@ -214,7 +214,7 @@ CHIP_ERROR AttributePathIB::Parser::GetListIndex(ConcreteDataAttributePath & aAt
else
{
// TODO: Add ListOperation::ReplaceItem support. (Attribute path with valid list index)
err = CHIP_ERROR_IM_MALFORMED_ATTRIBUTE_PATH;
err = CHIP_ERROR_IM_MALFORMED_ATTRIBUTE_PATH_IB;
}
}
else if (CHIP_END_OF_TLV == err)
Expand Down
14 changes: 7 additions & 7 deletions src/app/MessageDef/AttributeReportIB.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ namespace app {
CHIP_ERROR AttributeReportIB::Parser::CheckSchemaValidity() const
{
CHIP_ERROR err = CHIP_NO_ERROR;
int TagPresenceMask = 0;
int tagPresenceMask = 0;
TLV::TLVReader reader;

PRETTY_PRINT("AttributeReportIB =");
Expand All @@ -52,8 +52,8 @@ CHIP_ERROR AttributeReportIB::Parser::CheckSchemaValidity() const
{
case to_underlying(Tag::kAttributeStatus):
// check if this tag has appeared before
VerifyOrReturnError(!(TagPresenceMask & (1 << to_underlying(Tag::kAttributeStatus))), CHIP_ERROR_INVALID_TLV_TAG);
TagPresenceMask |= (1 << to_underlying(Tag::kAttributeStatus));
VerifyOrReturnError(!(tagPresenceMask & (1 << to_underlying(Tag::kAttributeStatus))), CHIP_ERROR_INVALID_TLV_TAG);
tagPresenceMask |= (1 << to_underlying(Tag::kAttributeStatus));
{
AttributeStatusIB::Parser attributeStatus;
ReturnErrorOnFailure(attributeStatus.Init(reader));
Expand All @@ -65,8 +65,8 @@ CHIP_ERROR AttributeReportIB::Parser::CheckSchemaValidity() const
break;
case to_underlying(Tag::kAttributeData):
// check if this tag has appeared before
VerifyOrReturnError(!(TagPresenceMask & (1 << to_underlying(Tag::kAttributeData))), CHIP_ERROR_INVALID_TLV_TAG);
TagPresenceMask |= (1 << to_underlying(Tag::kAttributeData));
VerifyOrReturnError(!(tagPresenceMask & (1 << to_underlying(Tag::kAttributeData))), CHIP_ERROR_INVALID_TLV_TAG);
tagPresenceMask |= (1 << to_underlying(Tag::kAttributeData));
{
AttributeDataIB::Parser attributeData;
ReturnErrorOnFailure(attributeData.Init(reader));
Expand All @@ -91,12 +91,12 @@ CHIP_ERROR AttributeReportIB::Parser::CheckSchemaValidity() const
const int CheckDataField = 1 << to_underlying(Tag::kAttributeData);
const int CheckStatusField = (1 << to_underlying(Tag::kAttributeStatus));

if ((TagPresenceMask & CheckDataField) == CheckDataField && (TagPresenceMask & CheckStatusField) == CheckStatusField)
if ((tagPresenceMask & CheckDataField) == CheckDataField && (tagPresenceMask & CheckStatusField) == CheckStatusField)
{
// kAttributeData and kAttributeStatus both exist
err = CHIP_ERROR_IM_MALFORMED_ATTRIBUTE_REPORT_IB;
}
else if ((TagPresenceMask & CheckDataField) != CheckDataField && (TagPresenceMask & CheckStatusField) != CheckStatusField)
else if ((tagPresenceMask & CheckDataField) != CheckDataField && (tagPresenceMask & CheckStatusField) != CheckStatusField)
{
// kPath and kErrorStatus not exist
err = CHIP_ERROR_IM_MALFORMED_ATTRIBUTE_REPORT_IB;
Expand Down
22 changes: 7 additions & 15 deletions src/app/MessageDef/AttributeStatusIB.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ namespace app {
CHIP_ERROR AttributeStatusIB::Parser::CheckSchemaValidity() const
{
CHIP_ERROR err = CHIP_NO_ERROR;
int TagPresenceMask = 0;
int tagPresenceMask = 0;
TLV::TLVReader reader;

PRETTY_PRINT("AttributeStatusIB =");
Expand All @@ -51,8 +51,8 @@ CHIP_ERROR AttributeStatusIB::Parser::CheckSchemaValidity() const
{
case to_underlying(Tag::kPath):
// check if this tag has appeared before
VerifyOrReturnError(!(TagPresenceMask & (1 << to_underlying(Tag::kPath))), CHIP_ERROR_INVALID_TLV_TAG);
TagPresenceMask |= (1 << to_underlying(Tag::kPath));
VerifyOrReturnError(!(tagPresenceMask & (1 << to_underlying(Tag::kPath))), CHIP_ERROR_INVALID_TLV_TAG);
tagPresenceMask |= (1 << to_underlying(Tag::kPath));
{
AttributePathIB::Parser path;
ReturnErrorOnFailure(path.Init(reader));
Expand All @@ -64,8 +64,8 @@ CHIP_ERROR AttributeStatusIB::Parser::CheckSchemaValidity() const
break;
case to_underlying(Tag::kErrorStatus):
// check if this tag has appeared before
VerifyOrReturnError(!(TagPresenceMask & (1 << to_underlying(Tag::kErrorStatus))), CHIP_ERROR_INVALID_TLV_TAG);
TagPresenceMask |= (1 << to_underlying(Tag::kErrorStatus));
VerifyOrReturnError(!(tagPresenceMask & (1 << to_underlying(Tag::kErrorStatus))), CHIP_ERROR_INVALID_TLV_TAG);
tagPresenceMask |= (1 << to_underlying(Tag::kErrorStatus));
{
StatusIB::Parser errorStatus;
ReturnErrorOnFailure(errorStatus.Init(reader));
Expand All @@ -86,16 +86,8 @@ CHIP_ERROR AttributeStatusIB::Parser::CheckSchemaValidity() const

if (CHIP_END_OF_TLV == err)
{
const int RequiredFields = (1 << to_underlying(Tag::kPath)) | (1 << to_underlying(Tag::kErrorStatus));

if ((TagPresenceMask & RequiredFields) == RequiredFields)
{
err = CHIP_NO_ERROR;
}
else
{
err = CHIP_ERROR_IM_MALFORMED_ATTRIBUTE_STATUS_IB;
}
const int requiredFields = (1 << to_underlying(Tag::kPath)) | (1 << to_underlying(Tag::kErrorStatus));
err = (tagPresenceMask & requiredFields) == requiredFields ? CHIP_NO_ERROR : CHIP_ERROR_IM_MALFORMED_ATTRIBUTE_STATUS_IB;
}

ReturnErrorOnFailure(err);
Expand Down
11 changes: 2 additions & 9 deletions src/app/MessageDef/ClusterPathIB.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -101,16 +101,9 @@ CHIP_ERROR ClusterPathIB::Parser::CheckSchemaValidity() const
if (CHIP_END_OF_TLV == err)
{
// check for required fields:
const int RequiredFields = (1 << to_underlying(Tag::kCluster));
const int requiredFields = (1 << to_underlying(Tag::kCluster));

if ((tagPresenceMask & RequiredFields) == RequiredFields)
{
err = CHIP_NO_ERROR;
}
else
{
err = CHIP_ERROR_IM_MALFORMED_CLUSTER_PATH_IB;
}
err = (tagPresenceMask & requiredFields) == requiredFields ? CHIP_NO_ERROR : CHIP_ERROR_IM_MALFORMED_CLUSTER_PATH_IB;
}

ReturnErrorOnFailure(err);
Expand Down
22 changes: 7 additions & 15 deletions src/app/MessageDef/CommandDataIB.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ CommandDataIB::Parser::ParseFields(TLV::TLVReader & aReader, int aDepth) const
CHIP_ERROR CommandDataIB::Parser::CheckSchemaValidity() const
{
CHIP_ERROR err = CHIP_NO_ERROR;
int TagPresenceMask = 0;
int tagPresenceMask = 0;
TLV::TLVReader reader;

PRETTY_PRINT("CommandDataIB =");
Expand All @@ -224,8 +224,8 @@ CHIP_ERROR CommandDataIB::Parser::CheckSchemaValidity() const
{
case to_underlying(Tag::kPath):
// check if this tag has appeared before
VerifyOrReturnError(!(TagPresenceMask & (1 << to_underlying(Tag::kPath))), CHIP_ERROR_INVALID_TLV_TAG);
TagPresenceMask |= (1 << to_underlying(Tag::kPath));
VerifyOrReturnError(!(tagPresenceMask & (1 << to_underlying(Tag::kPath))), CHIP_ERROR_INVALID_TLV_TAG);
tagPresenceMask |= (1 << to_underlying(Tag::kPath));
{
CommandPathIB::Parser path;
ReturnErrorOnFailure(path.Init(reader));
Expand All @@ -237,8 +237,8 @@ CHIP_ERROR CommandDataIB::Parser::CheckSchemaValidity() const
break;
case to_underlying(Tag::kFields):
// check if this tag has appeared before
VerifyOrReturnError(!(TagPresenceMask & (1 << to_underlying(Tag::kFields))), CHIP_ERROR_INVALID_TLV_TAG);
TagPresenceMask |= (1 << to_underlying(Tag::kFields));
VerifyOrReturnError(!(tagPresenceMask & (1 << to_underlying(Tag::kFields))), CHIP_ERROR_INVALID_TLV_TAG);
tagPresenceMask |= (1 << to_underlying(Tag::kFields));
ReturnErrorOnFailure(ParseFields(reader, 0));
break;
default:
Expand All @@ -252,16 +252,8 @@ CHIP_ERROR CommandDataIB::Parser::CheckSchemaValidity() const

if (CHIP_END_OF_TLV == err)
{
const int RequiredFields = 1 << to_underlying(Tag::kPath);

if ((TagPresenceMask & RequiredFields) == RequiredFields)
{
err = CHIP_NO_ERROR;
}
else
{
err = CHIP_ERROR_IM_MALFORMED_COMMAND_DATA_IB;
}
const int requiredFields = 1 << to_underlying(Tag::kPath);
err = (tagPresenceMask & requiredFields) == requiredFields ? CHIP_NO_ERROR : CHIP_ERROR_IM_MALFORMED_COMMAND_DATA_IB;
}

ReturnErrorOnFailure(err);
Expand Down
Loading

0 comments on commit 4d230f8

Please sign in to comment.