Skip to content

Commit

Permalink
Remove raw value access from TLV::Tag (#27869)
Browse files Browse the repository at this point in the history 
* Provide a AppendTo for formatting TLV tags

* Restyle

* Fix uni tests to new formatting

* Restyle

* Fix tag formatting when enter container fails
  • Loading branch information
@andy31415 @pull
andy31415 authored and pull[bot] committed Sep 21, 2023
1 parent 6fafec9 commit 093bc7f
Show file tree
Hide file tree
Showing 5 changed files with 109 additions and 105 deletions.
1 change: 1 addition & 0 deletions src/lib/core/BUILD.gn
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ static_library("core") {
"TLVCircularBuffer.h",
"TLVDebug.cpp",
"TLVReader.cpp",
"TLVTags.cpp",
"TLVTags.h",
"TLVTypes.h",
"TLVUpdater.cpp",
Expand Down
48 changes: 48 additions & 0 deletions src/lib/core/TLVTags.cpp
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
/*
*
* Copyright (c) 2020-2021 Project CHIP Authors
* All rights reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
#include "TLVTags.h"

namespace chip {
namespace TLV {

/// Appends the text representation for a tag to the given string builder base.
StringBuilderBase & Tag::AppendTo(StringBuilderBase & out)
{
if (TLV::IsProfileTag(*this))
{
out.AddFormat("ProfileTag(0x%X::0x%X::0x%" PRIX32 ")", TLV::VendorIdFromTag(*this), TLV::ProfileNumFromTag(*this),
TLV::TagNumFromTag(*this));
}
else if (TLV::IsContextTag(*this))
{
out.AddFormat("ContextTag(0x%" PRIX32 ")", TLV::TagNumFromTag(*this));
}
else if (*this == TLV::AnonymousTag())
{
out.Add("AnonymousTag()");
}
else
{
out.AddFormat("UnknownTag(0x%" PRIX64 ")", mVal);
}

return out;
}

} // namespace TLV
} // namespace chip
4 changes: 3 additions & 1 deletion src/lib/core/TLVTags.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#pragma once

#include <cstdint>
#include <lib/support/StringBuilder.h>
#include <lib/support/TypeTraits.h>
#include <type_traits>

Expand All @@ -46,7 +47,8 @@ class Tag
constexpr bool operator==(const Tag & other) const { return mVal == other.mVal; }
constexpr bool operator!=(const Tag & other) const { return mVal != other.mVal; }

uint64_t RawValue() const { return mVal; }
/// Appends the text representation of the tag to the given string builder base.
StringBuilderBase & AppendTo(StringBuilderBase & out);

private:
explicit constexpr Tag(uint64_t val) : mVal(val) {}
Expand Down
61 changes: 7 additions & 54 deletions src/lib/format/protocol_decoder.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,51 +45,6 @@ class ByTag
const Tag mTag;
};

const char * DecodeTagControl(const TLVTagControl aTagControl)
{
switch (aTagControl)
{
case TLVTagControl::Anonymous:
return "Anonymous";
case TLVTagControl::ContextSpecific:
return "ContextSpecific";
case TLVTagControl::CommonProfile_2Bytes:
return "Common2B";
case TLVTagControl::CommonProfile_4Bytes:
return "Common4B";
case TLVTagControl::ImplicitProfile_2Bytes:
return "Implicit2B";
case TLVTagControl::ImplicitProfile_4Bytes:
return "Implicit4B";
case TLVTagControl::FullyQualified_6Bytes:
return "FullyQualified6B";
case TLVTagControl::FullyQualified_8Bytes:
return "FullyQualified8";
default:
return "???";
}
}

void FormatCurrentTag(const TLVReader & reader, chip::StringBuilderBase & out)
{
chip::TLV::TLVTagControl tagControl = static_cast<TLVTagControl>(reader.GetControlByte() & kTLVTagControlMask);
chip::TLV::Tag tag = reader.GetTag();

if (IsProfileTag(tag))
{
out.AddFormat("%s(0x%X::0x%X::0x%" PRIX32 ")", DecodeTagControl(tagControl), VendorIdFromTag(tag), ProfileNumFromTag(tag),
TagNumFromTag(tag));
}
else if (IsContextTag(tag))
{
out.AddFormat("%s(0x%" PRIX32 ")", DecodeTagControl(tagControl), TagNumFromTag(tag));
}
else
{
out.AddFormat("UnknownTag(0x%" PRIX64 ")", tag.RawValue());
}
}

CHIP_ERROR FormatCurrentValue(TLVReader & reader, chip::StringBuilderBase & out)
{
switch (reader.GetType())
Expand Down Expand Up @@ -351,6 +306,7 @@ bool PayloadDecoderBase::ReaderEnterContainer(PayloadEntry & entry)
if (mCurrentNesting >= kMaxDecodeDepth)
{
mValueBuilder.AddFormat("NESTING DEPTH REACHED");
mReader.GetTag().AppendTo(mNameBuilder.Reset());
entry = PayloadEntry::SimpleValue(mNameBuilder.c_str(), mValueBuilder.c_str());
return false;
}
Expand All @@ -360,6 +316,7 @@ bool PayloadDecoderBase::ReaderEnterContainer(PayloadEntry & entry)
if (err != CHIP_NO_ERROR)
{
mValueBuilder.AddFormat("ERROR entering container: %" CHIP_ERROR_FORMAT, err.Format());
mReader.GetTag().AppendTo(mNameBuilder.Reset()); // assume enter is not done, so tag is correct
entry = PayloadEntry::SimpleValue(mNameBuilder.c_str(), mValueBuilder.c_str());
mState = State::kDone;
return false;
Expand All @@ -372,13 +329,8 @@ bool PayloadDecoderBase::ReaderEnterContainer(PayloadEntry & entry)

void PayloadDecoderBase::EnterContainer(PayloadEntry & entry)
{
// must be done BEFORE entering container
// to preserve the value and not get a 'container tag'
// below when data is not valid
//
// TODO: this formatting is wasteful, should really be done only
// if data is NULLPTR.
FormatCurrentTag(mReader, mNameBuilder.Reset());
// Tag fetch must be done BEFORE entering container
chip::TLV::Tag tag = mReader.GetTag();

VerifyOrReturn(ReaderEnterContainer(entry));

Expand All @@ -396,6 +348,7 @@ void PayloadDecoderBase::EnterContainer(PayloadEntry & entry)

if (data == nullptr)
{
tag.AppendTo(mNameBuilder.Reset());
entry = PayloadEntry::NestingEnter(mNameBuilder.c_str());
}
else
Expand Down Expand Up @@ -454,7 +407,7 @@ void PayloadDecoderBase::NextFromContentRead(PayloadEntry & entry)

if (data == nullptr)
{
FormatCurrentTag(mReader, mNameBuilder.Reset());
mReader.GetTag().AppendTo(mNameBuilder.Reset());
entry = PayloadEntry::SimpleValue(mNameBuilder.c_str(), mValueBuilder.c_str());
return;
}
Expand Down Expand Up @@ -600,7 +553,7 @@ void PayloadDecoderBase::NextFromValueRead(PayloadEntry & entry)

if (data == nullptr)
{
FormatCurrentTag(mReader, mNameBuilder.Reset());
mReader.GetTag().AppendTo(mNameBuilder.Reset());
PrettyPrintCurrentValue(mReader, mValueBuilder.Reset(), mPayloadPosition);
entry = PayloadEntry::SimpleValue(mNameBuilder.c_str(), mValueBuilder.c_str());
mPayloadPosition.Exit();
Expand Down
100 changes: 50 additions & 50 deletions src/lib/format/tests/TestDecoding.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -505,14 +505,14 @@ void TestEmptyClusterMetaDataDecode(nlTestSuite * inSuite, void * inContext)
" endpoint_id: 0\n"
" cluster_id: 31\n"
" attribute_id: 0\n"
" 0x1f::ATTR(0x0)\n" // Cluster 31, attribute 0
" UnknownTag(0x100)\n" // List entry (acl is a list)
" ContextSpecific(0x1): 5\n" // privilege
" ContextSpecific(0x2): 2\n" // authMode
" ContextSpecific(0x3)\n" // subjects
" UnknownTag(0x100): 112233\n" // List entry (subjects is a list)
" ContextSpecific(0x4): NULL\n" // targets
" ContextSpecific(0xFE): 1\n" // fabricIndex
" 0x1f::ATTR(0x0)\n" // Cluster 31, attribute 0
" AnonymousTag()\n" // List entry (acl is a list)
" ContextTag(0x1): 5\n" // privilege
" ContextTag(0x2): 2\n" // authMode
" ContextTag(0x3)\n" // subjects
" AnonymousTag(): 112233\n" // List entry (subjects is a list)
" ContextTag(0x4): NULL\n" // targets
" ContextTag(0xFE): 1\n" // fabricIndex
" suppress_response: true\n"
" interaction_model_revison: 1\n");
}
Expand All @@ -536,24 +536,24 @@ void TestWrongDecodeData(nlTestSuite * inSuite, void * inContext)
TestSampleData(inSuite, params, secure_channel_mrp_ack, "proto16: EMPTY\n");
TestSampleData(inSuite, params, im_protocol_report_data_acl,
"proto5\n"
" ContextSpecific(0x1)\n"
" UnknownTag(0x100)\n"
" ContextSpecific(0x1)\n"
" ContextSpecific(0x0): 3420147058\n"
" ContextSpecific(0x1)\n"
" ContextSpecific(0x2): 0\n"
" ContextSpecific(0x3): 31\n"
" ContextSpecific(0x4): 0\n"
" ContextSpecific(0x2)\n"
" UnknownTag(0x100)\n"
" ContextSpecific(0x1): 5\n"
" ContextSpecific(0x2): 2\n"
" ContextSpecific(0x3)\n"
" UnknownTag(0x100): 112233\n"
" ContextSpecific(0x4): NULL\n"
" ContextSpecific(0xFE): 1\n"
" ContextSpecific(0x4): true\n"
" ContextSpecific(0xFF): 1\n");
" ContextTag(0x1)\n"
" AnonymousTag()\n"
" ContextTag(0x1)\n"
" ContextTag(0x0): 3420147058\n"
" ContextTag(0x1)\n"
" ContextTag(0x2): 0\n"
" ContextTag(0x3): 31\n"
" ContextTag(0x4): 0\n"
" ContextTag(0x2)\n"
" AnonymousTag()\n"
" ContextTag(0x1): 5\n"
" ContextTag(0x2): 2\n"
" ContextTag(0x3)\n"
" AnonymousTag(): 112233\n"
" ContextTag(0x4): NULL\n"
" ContextTag(0xFE): 1\n"
" ContextTag(0x4): true\n"
" ContextTag(0xFF): 1\n");
}

void TestNestingOverflow(nlTestSuite * inSuite, void * inContext)
Expand Down Expand Up @@ -629,30 +629,30 @@ void TestNestingOverflow(nlTestSuite * inSuite, void * inContext)

TestSampleData(inSuite, params, fake_payload,
"proto5\n"
" ContextSpecific(0x0)\n"
" ContextSpecific(0x1)\n"
" ContextSpecific(0x2)\n"
" ContextSpecific(0x3)\n"
" ContextSpecific(0x4)\n"
" ContextSpecific(0x5)\n"
" ContextSpecific(0x6)\n"
" ContextSpecific(0x7)\n"
" ContextSpecific(0x8)\n"
" ContextSpecific(0x9)\n"
" ContextSpecific(0xA)\n"
" ContextSpecific(0xB)\n"
" ContextSpecific(0xC)\n"
" ContextSpecific(0xD)\n"
" ContextSpecific(0xE)\n"
" ContextSpecific(0xF): NESTING DEPTH REACHED\n"
" ContextSpecific(0x20)\n"
" ContextSpecific(0x21)\n"
" ContextSpecific(0x22)\n"
" ContextSpecific(0x23)\n"
" ContextSpecific(0x30)\n"
" ContextSpecific(0x31)\n"
" ContextSpecific(0x32)\n"
" ContextSpecific(0x33)\n");
" ContextTag(0x0)\n"
" ContextTag(0x1)\n"
" ContextTag(0x2)\n"
" ContextTag(0x3)\n"
" ContextTag(0x4)\n"
" ContextTag(0x5)\n"
" ContextTag(0x6)\n"
" ContextTag(0x7)\n"
" ContextTag(0x8)\n"
" ContextTag(0x9)\n"
" ContextTag(0xA)\n"
" ContextTag(0xB)\n"
" ContextTag(0xC)\n"
" ContextTag(0xD)\n"
" ContextTag(0xE)\n"
" ContextTag(0xF): NESTING DEPTH REACHED\n"
" ContextTag(0x20)\n"
" ContextTag(0x21)\n"
" ContextTag(0x22)\n"
" ContextTag(0x23)\n"
" ContextTag(0x30)\n"
" ContextTag(0x31)\n"
" ContextTag(0x32)\n"
" ContextTag(0x33)\n");
}

const nlTest sTests[] = {
Expand Down

0 comments on commit 093bc7f

Please sign in to comment.