Code review

project-chip · Jan 4, 2023 · 72e9d23 · 72e9d23
1 parent 507b494
commit 72e9d23
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 26 deletions.
diff --git a/src/lib/core/CHIPTLVTags.h b/src/lib/core/CHIPTLVTags.h
@@ -32,6 +32,13 @@ namespace TLV {
 class Tag
 {
 public:
+    enum SpecialTagNumber : uint32_t
+    {
+        kContextTagMaxNum = UINT8_MAX,
+        kAnonymousTagNum,
+        kUnknownImplicitTagNum
+    };
+
     Tag() = default;
 
     constexpr bool operator==(const Tag & other) const { return mVal == other.mVal; }
@@ -66,22 +73,15 @@ class Tag
     static constexpr uint32_t kProfileNumShift     = 32;
     static constexpr uint32_t kSpecialTagProfileId = 0xFFFFFFFF;
 
-    enum SpecialTagNumber : uint32_t
-    {
-        kContextTagMaxNum = UINT8_MAX,
-        kAnonymousTagNum,
-        kUnknownImplicitTagNum
-    };
-
-    // The API representation of the tag uses the following encoding:
+    // The storage of the tag value uses the following encoding:
     //
-    //  63                     47                       31
-    // +-----------------------+-----------------------+----------------------------------------------+
-    // | Vendor id (negated)   | Profile num (negated) | Tag number                                   |
-    // +-----------------------+-----------------------+----------------------------------------------+
+    //  63                              47                              31
+    // +-------------------------------+-------------------------------+----------------------------------------------+
+    // | Vendor id (bitwise-negated)   | Profile num (bitwise-negated) | Tag number                                   |
+    // +-------------------------------+-------------------------------+----------------------------------------------+
     //
-    // Vendor id and profile number are negated in order to optimize the code size when using
-    // context tags, the most commonly used tags in the SDK.
+    // Vendor id and profile number are bitwise-negated in order to optimize the code size when
+    // using context tags, the most commonly used tags in the SDK.
     uint64_t mVal;
 };
 

diff --git a/src/lib/core/CHIPTLVWriter.cpp b/src/lib/core/CHIPTLVWriter.cpp
@@ -629,21 +629,24 @@ CHIP_ERROR TLVWriter::WriteElementHead(TLVElementType elemType, Tag tag, uint64_
     else
         p = stagingBuf;
 
-    if (IsContextTag(tag))
+    if (IsSpecialTag(tag))
     {
-        if (mContainerType != kTLVType_Structure && mContainerType != kTLVType_List)
-            return CHIP_ERROR_INVALID_TLV_TAG;
+        if (tagNum <= Tag::kContextTagMaxNum)
+        {
+            if (mContainerType != kTLVType_Structure && mContainerType != kTLVType_List)
+                return CHIP_ERROR_INVALID_TLV_TAG;
 
-        Write8(p, TLVTagControl::ContextSpecific | elemType);
-        Write8(p, static_cast<uint8_t>(tagNum));
-    }
-    else if (IsSpecialTag(tag))
-    {
-        if (elemType != TLVElementType::EndOfContainer && mContainerType != kTLVType_NotSpecified &&
-            mContainerType != kTLVType_Array && mContainerType != kTLVType_List)
-            return CHIP_ERROR_INVALID_TLV_TAG;
+            Write8(p, TLVTagControl::ContextSpecific | elemType);
+            Write8(p, static_cast<uint8_t>(tagNum));
+        }
+        else
+        {
+            if (elemType != TLVElementType::EndOfContainer && mContainerType != kTLVType_NotSpecified &&
+                mContainerType != kTLVType_Array && mContainerType != kTLVType_List)
+                return CHIP_ERROR_INVALID_TLV_TAG;
 
-        Write8(p, TLVTagControl::Anonymous | elemType);
+            Write8(p, TLVTagControl::Anonymous | elemType);
+        }
     }
     else
     {