Clang tidy enable clang-analyzer-core.UndefinedBinaryOperatorResult (p…

…roject-chip#17649) * Clang tidy enable clang-analyzer-core.UndefinedBinaryOperatorResult * Restyle * Update src/app/util/util.cpp Co-authored-by: Boris Zbarsky <[email protected]> * Make some tests pass tidy (missing initializes and usage of NL_TEST_ASSERT confuses tidy) Co-authored-by: Boris Zbarsky <[email protected]>
antonio-amjr · Apr 25, 2022 · 710cc81 · 710cc81
1 parent c292484
commit 710cc81
Show file tree

Hide file tree

Showing 5 changed files with 17 additions and 7 deletions.
diff --git a/.clang-tidy b/.clang-tidy
@@ -23,7 +23,6 @@ Checks: >
   -bugprone-signed-char-misuse,
   -bugprone-copy-constructor-init,
   -clang-analyzer-core.CallAndMessage,
-  -clang-analyzer-core.UndefinedBinaryOperatorResult,
   -clang-analyzer-core.NullDereference,
   -clang-analyzer-optin.cplusplus.UninitializedObject,
   -clang-analyzer-optin.performance,

diff --git a/src/access/tests/TestAccessControl.cpp b/src/access/tests/TestAccessControl.cpp
@@ -1984,11 +1984,11 @@ void TestSubjectsTargets(nlTestSuite * inSuite, void * inContext)
     NL_TEST_ASSERT(inSuite, accessControl.CreateEntry(&index, entry) == CHIP_NO_ERROR);
     NL_TEST_ASSERT(inSuite, int(index) == 2);
 
-    FabricIndex fabricIndex;
-    Privilege privilege;
-    AuthMode authMode;
-    size_t count;
-    NodeId subject;
+    FabricIndex fabricIndex = 0;
+    Privilege privilege     = Privilege::kView;
+    AuthMode authMode       = AuthMode::kNone;
+    size_t count            = 0;
+    NodeId subject          = kUndefinedNodeId;
     Target target;
 
     NL_TEST_ASSERT(inSuite, accessControl.ReadEntry(0, entry) == CHIP_NO_ERROR);

diff --git a/src/app/tests/TestClusterStateCache.cpp b/src/app/tests/TestClusterStateCache.cpp
@@ -313,7 +313,7 @@ class CacheValidator : public ClusterStateCache::Callback
         case AttributeInstruction::kAttributeA: {
             ChipLogProgress(DataManagement, "\t\t -- Validating A");
 
-            Clusters::TestCluster::Attributes::Int16u::TypeInfo::DecodableType v;
+            Clusters::TestCluster::Attributes::Int16u::TypeInfo::DecodableType v = 0;
             err = cache->Get<Clusters::TestCluster::Attributes::Int16u::TypeInfo>(path, v);
             if (err == CHIP_ERROR_IM_STATUS_CODE_RECEIVED)
             {

diff --git a/src/app/util/util.cpp b/src/app/util/util.cpp
@@ -515,6 +515,11 @@ void emberAfCopyLongString(uint8_t * dest, const uint8_t * src, size_t size)
 // default value of NULL is treated as all zeroes.
 int8_t emberAfCompareValues(const uint8_t * val1, const uint8_t * val2, uint16_t len, bool signedNumber)
 {
+    if (len == 0)
+    {
+        // no length means nothing to compare.  Shouldn't even happen, since len is sizeof(some-integer-type).
+        return 0;
+    }
     uint8_t i, j, k;
     if (signedNumber)
     { // signed number comparison

diff --git a/src/lib/asn1/ASN1Reader.cpp b/src/lib/asn1/ASN1Reader.cpp
@@ -147,12 +147,18 @@ CHIP_ERROR ASN1Reader::GetInteger(int64_t & val)
     ReturnErrorCodeIf(ValueLen > sizeof(int64_t), ASN1_ERROR_VALUE_OVERFLOW);
     ReturnErrorCodeIf(mElemStart + mHeadLen + ValueLen > mContainerEnd, ASN1_ERROR_UNDERRUN);
 
+    // NOLINTBEGIN(clang-analyzer-core.UndefinedBinaryOperatorResult)
+    //
+    // TODO: clang-tidy says that if val is -1, then (val << 8) is not defined.
+    //       added above supression to keep clang-tidy validating the rest of the files, however
+    //       this complain likely needs fixing.
     const uint8_t * p = Value;
     val               = ((*p & 0x80) == 0) ? 0 : -1;
     for (uint32_t i = ValueLen; i > 0; i--, p++)
     {
         val = (val << 8) | *p;
     }
+    // NOLINTEND(clang-analyzer-core.UndefinedBinaryOperatorResult)
 
     return CHIP_NO_ERROR;
 }