nodiscard ChipError Batch project-chip#7: access, ble, credentials (project-chip#41865)

* Batch project-chip#7 nodiscard errors: access, ble, credentials

* Clang tidy

Author: gmarcosb

src/access/examples/ExampleAccessControlDelegate.cpp

@@ -602,7 +602,7 @@ class EntryDelegate : public Entry::Delegate
     {
         ReturnErrorOnFailure(EnsureStorageInPool());
         size_t count = 0;
-        GetSubjectCount(count);
+        TEMPORARY_RETURN_IGNORED GetSubjectCount(count);
         if (count < EntryStorage::kMaxSubjects)
         {
             CHIP_ERROR err = mStorage->mSubjects[count].Add(subject);
@@ -619,7 +619,7 @@ class EntryDelegate : public Entry::Delegate
     {
         ReturnErrorOnFailure(EnsureStorageInPool());
         size_t count = 0;
-        GetSubjectCount(count);
+        TEMPORARY_RETURN_IGNORED GetSubjectCount(count);
         if (index < count)
         {
             // The storage at the specified index will be deleted by copying any subsequent storage
@@ -675,7 +675,7 @@ class EntryDelegate : public Entry::Delegate
     {
         ReturnErrorOnFailure(EnsureStorageInPool());
         size_t count = 0;
-        GetTargetCount(count);
+        TEMPORARY_RETURN_IGNORED GetTargetCount(count);
         if (count < EntryStorage::kMaxTargets)
         {
             CHIP_ERROR err = mStorage->mTargets[count].Add(target);
@@ -692,7 +692,7 @@ class EntryDelegate : public Entry::Delegate
     {
         ReturnErrorOnFailure(EnsureStorageInPool());
         size_t count = 0;
-        GetTargetCount(count);
+        TEMPORARY_RETURN_IGNORED GetTargetCount(count);
         if (index < count)
         {
             // The storage at the specified index will be deleted by copying any subsequent storage
@@ -732,7 +732,7 @@ class EntryDelegate : public Entry::Delegate
         if (mStorage == &storage)
         {
             // Best effort, OK if it fails.
-            EnsureStorageInPool();
+            TEMPORARY_RETURN_IGNORED EnsureStorageInPool();
         }
     }
 

src/ble/BLEEndPoint.cpp

@@ -445,7 +445,7 @@ void BLEEndPoint::ReleaseBleConnection()
         if (mConnStateFlags.Has(ConnectionStateFlag::kAutoClose))
         {
             ChipLogProgress(Ble, "Auto-closing end point's BLE connection.");
-            mBle->mPlatformDelegate->CloseConnection(mConnObj);
+            TEMPORARY_RETURN_IGNORED mBle->mPlatformDelegate->CloseConnection(mConnObj);
         }
         else
         {
@@ -876,7 +876,7 @@ CHIP_ERROR BLEEndPoint::DoSendStandAloneAck()
     ChipLogDebugBleEndPoint(Ble, "entered DoSendStandAloneAck; sending stand-alone ack");
 
     // Encode and transmit stand-alone ack.
-    mBtpEngine.EncodeStandAloneAck(mAckToSend);
+    ReturnErrorOnFailure(mBtpEngine.EncodeStandAloneAck(mAckToSend));
     ReturnErrorOnFailure(SendCharacteristic(mAckToSend.Retain()));
 
     // Reset local receive window counter.

src/ble/BleLayer.cpp

@@ -431,7 +431,7 @@ CHIP_ERROR BleLayer::NewBleEndPoint(BLEEndPoint ** retEndPoint, BLE_CONNECTION_O
         return CHIP_ERROR_ENDPOINT_POOL_FULL;
     }
 
-    endPoint->Init(this, connObj, role, autoClose);
+    TEMPORARY_RETURN_IGNORED endPoint->Init(this, connObj, role, autoClose);
     endPoint->mBleTransport = mBleTransport;
 
     *retEndPoint = endPoint;

src/credentials/CHIPCert.cpp

@@ -1489,7 +1489,7 @@ void InitNetworkIdentitySubject(ChipDN & name)
 {
     name.Clear();
     CHIP_ERROR err = name.AddAttribute_CommonName(kNetworkIdentityCN, /* not printable */ false);
-    VerifyOrDie(err == CHIP_NO_ERROR); // AddAttribute can't fail in this case
+    SuccessOrDie(err); // AddAttribute can't fail in this case
 }
 
 static CHIP_ERROR CalculateKeyIdentifierSha256(const P256PublicKeySpan & publicKey, MutableCertificateKeyId outKeyId)

src/credentials/CHIPCertToX509.cpp

@@ -603,17 +603,18 @@ static CHIP_ERROR DecodeConvertCert(TLVReader & reader, ASN1Writer & writer, ASN
     ASN1_START_SEQUENCE
     {
         // tbsCertificate TBSCertificate,
-        reader.Next();
+        TEMPORARY_RETURN_IGNORED reader.Next();
         if (reader.GetTag() == ContextTag(kTag_EllipticCurvePublicKey))
         {
             // If the struct starts with the ec-pub-key we're dealing with a
             // Network (Client) Identity in compact-pdc-identity format.
-            DecodeConvertTBSCertCompactIdentity(reader, tbsWriter, certData);
+            err = DecodeConvertTBSCertCompactIdentity(reader, tbsWriter, certData);
         }
         else
         {
-            ReturnErrorOnFailure(DecodeConvertTBSCert(reader, tbsWriter, certData));
+            err = DecodeConvertTBSCert(reader, tbsWriter, certData);
         }
+        ReturnErrorOnFailure(err);
 
         // signatureAlgorithm   AlgorithmIdentifier
         // AlgorithmIdentifier ::= SEQUENCE

src/credentials/FabricTable.cpp

@@ -154,7 +154,7 @@ void FabricInfo::operator=(FabricInfo && other)
     mVendorId                = other.mVendorId;
     mShouldAdvertiseIdentity = other.mShouldAdvertiseIdentity;
 
-    SetFabricLabel(other.GetFabricLabel());
+    TEMPORARY_RETURN_IGNORED SetFabricLabel(other.GetFabricLabel());
 
     // Transfer ownership of operational keypair (if it was nullptr, it stays that way).
     mOperationalKey                         = other.mOperationalKey;
@@ -911,7 +911,7 @@ FabricTable::AddOrUpdateInner(FabricIndex fabricIndex, bool isAddition, Crypto::
     ReturnErrorOnFailure(fabricEntry->Init(newFabricInfo));
 
     // Set the label, matching add/update semantics of empty/existing.
-    fabricEntry->SetFabricLabel(fabricLabel);
+    TEMPORARY_RETURN_IGNORED fabricEntry->SetFabricLabel(fabricLabel);
 
     if (isAddition)
     {
@@ -1018,7 +1018,7 @@ CHIP_ERROR FabricTable::Delete(FabricIndex fabricIndex)
         // If StoreFabricIndexInfo fails here, that's probably OK.  When we try to
         // read things from storage later we will realize there is nothing for this
         // index.
-        StoreFabricIndexInfo();
+        TEMPORARY_RETURN_IGNORED StoreFabricIndexInfo();
 
         // If we ever start moving the FabricInfo entries around in the array on
         // delete, we should update DeleteAllFabrics to handle that.
@@ -1067,7 +1067,7 @@ void FabricTable::DeleteAllFabrics()
 
     for (auto & fabric : *this)
     {
-        Delete(fabric.GetFabricIndex());
+        TEMPORARY_RETURN_IGNORED Delete(fabric.GetFabricIndex());
     }
 }
 
@@ -1096,7 +1096,7 @@ CHIP_ERROR FabricTable::Init(const FabricTable::InitParams & initParams)
     // Time is unknown during incoming certificate validation for CASE and
     // current time is also unknown, the certificate validity policy will see
     // this condition and can act appropriately.
-    mLastKnownGoodTime.Init(mStorage);
+    TEMPORARY_RETURN_IGNORED mLastKnownGoodTime.Init(mStorage);
 
     uint8_t buf[IndexInfoTLVMaxSize()];
     uint16_t size  = sizeof(buf);
@@ -1328,19 +1328,19 @@ CHIP_ERROR FabricTable::StoreFabricIndexInfo() const
 
     if (mNextAvailableFabricIndex.HasValue())
     {
-        writer.Put(kNextAvailableFabricIndexTag, mNextAvailableFabricIndex.Value());
+        TEMPORARY_RETURN_IGNORED writer.Put(kNextAvailableFabricIndexTag, mNextAvailableFabricIndex.Value());
     }
     else
     {
-        writer.PutNull(kNextAvailableFabricIndexTag);
+        TEMPORARY_RETURN_IGNORED writer.PutNull(kNextAvailableFabricIndexTag);
     }
 
     TLV::TLVType innerContainerType;
     ReturnErrorOnFailure(writer.StartContainer(kFabricIndicesTag, TLV::kTLVType_Array, innerContainerType));
     // Only enumerate the fabrics that are initialized.
     for (const auto & fabric : *this)
     {
-        writer.Put(TLV::AnonymousTag(), fabric.GetFabricIndex());
+        TEMPORARY_RETURN_IGNORED writer.Put(TLV::AnonymousTag(), fabric.GetFabricIndex());
     }
     ReturnErrorOnFailure(writer.EndContainer(innerContainerType));
     ReturnErrorOnFailure(writer.EndContainer(outerType));
@@ -1500,7 +1500,7 @@ CHIP_ERROR FabricTable::GetCommitMarker(CommitMarker & outCommitMarker)
 
 void FabricTable::ClearCommitMarker()
 {
-    mStorage->SyncDeleteKeyValue(DefaultStorageKeyAllocator::FabricTableCommitMarkerKey().KeyName());
+    TEMPORARY_RETURN_IGNORED mStorage->SyncDeleteKeyValue(DefaultStorageKeyAllocator::FabricTableCommitMarkerKey().KeyName());
 }
 
 bool FabricTable::HasOperationalKeyForFabric(FabricIndex fabricIndex) const
@@ -1760,7 +1760,7 @@ CHIP_ERROR FabricTable::AddNewPendingFabricCommon(const ByteSpan & noc, const By
 
     // Notify that NOC was added (at least transiently)
     *outNewFabricIndex = fabricIndexToUse;
-    NotifyFabricUpdated(fabricIndexToUse);
+    TEMPORARY_RETURN_IGNORED NotifyFabricUpdated(fabricIndexToUse);
 
     return CHIP_NO_ERROR;
 }
@@ -1845,7 +1845,7 @@ CHIP_ERROR FabricTable::UpdatePendingFabricCommon(FabricIndex fabricIndex, const
     mStateFlags.Set(StateFlags::kIsPendingFabricDataPresent);
 
     // Notify that NOC was updated (at least transiently)
-    NotifyFabricUpdated(fabricIndex);
+    TEMPORARY_RETURN_IGNORED NotifyFabricUpdated(fabricIndex);
 
     return CHIP_NO_ERROR;
 }
@@ -2061,13 +2061,13 @@ CHIP_ERROR FabricTable::CommitPendingFabricData()
     {
         // Blow-away everything if we got past any storage, even on Update: system state is broken
         // TODO: Develop a way to properly revert in the future, but this is very difficult
-        Delete(fabricIndexBeingCommitted);
+        TEMPORARY_RETURN_IGNORED Delete(fabricIndexBeingCommitted);
 
         RevertPendingFabricData();
     }
     else
     {
-        NotifyFabricCommitted(fabricIndexBeingCommitted);
+        TEMPORARY_RETURN_IGNORED NotifyFabricCommitted(fabricIndexBeingCommitted);
     }
 
     // Clear commit marker no matter what: if we got here, there was no reboot and previous clean-ups
@@ -2094,7 +2094,7 @@ void FabricTable::RevertPendingFabricData()
         mOpCertStore->RevertPendingOpCerts();
     }
 
-    mLastKnownGoodTime.RevertPendingLastKnownGoodChipEpochTime();
+    TEMPORARY_RETURN_IGNORED mLastKnownGoodTime.RevertPendingLastKnownGoodChipEpochTime();
 
     mStateFlags.ClearAll();
     mFabricIndexWithPendingState = kUndefinedFabricIndex;
@@ -2119,7 +2119,7 @@ void FabricTable::RevertPendingOpCertsExceptRoot()
     if (mStateFlags.Has(StateFlags::kIsAddPending))
     {
         // If we have a pending add, let's make sure to kill the pending fabric metadata and return it to viable state.
-        Delete(mFabricIndexWithPendingState);
+        TEMPORARY_RETURN_IGNORED Delete(mFabricIndexWithPendingState);
     }
 
     mStateFlags.Clear(StateFlags::kIsAddPending);

src/credentials/FabricTable.h

@@ -439,7 +439,7 @@ class DLL_EXPORT FabricTable
     // TODO this #if CONFIG_BUILD_FOR_HOST_UNIT_TEST is temporary. There is a change incoming soon
     // that will allow triggering NOC update directly.
 #if CONFIG_BUILD_FOR_HOST_UNIT_TEST
-    void SendUpdateFabricNotificationForTest(FabricIndex fabricIndex) { NotifyFabricUpdated(fabricIndex); }
+    void SendUpdateFabricNotificationForTest(FabricIndex fabricIndex) { TEMPORARY_RETURN_IGNORED NotifyFabricUpdated(fabricIndex); }
 #endif // CONFIG_BUILD_FOR_HOST_UNIT_TEST
 
     /**

src/credentials/GenerateChipX509Cert.cpp

@@ -423,9 +423,10 @@ CHIP_ERROR EncodeNetworkIdentityTBSCert(const P256PublicKey & pubkey, ASN1Writer
         {
             ASN1_START_SEQUENCE
             {
-                EncodeIsCAExtension(kNotCACert, writer);
-                EncodeKeyUsageExtension(KeyUsageFlags::kDigitalSignature, writer);
-                EncodeExtKeyUsageExtension({ kOID_KeyPurpose_ClientAuth, kOID_KeyPurpose_ServerAuth }, writer);
+                TEMPORARY_RETURN_IGNORED EncodeIsCAExtension(kNotCACert, writer);
+                TEMPORARY_RETURN_IGNORED EncodeKeyUsageExtension(KeyUsageFlags::kDigitalSignature, writer);
+                TEMPORARY_RETURN_IGNORED EncodeExtKeyUsageExtension({ kOID_KeyPurpose_ClientAuth, kOID_KeyPurpose_ServerAuth },
+                                                                    writer);
             }
             ASN1_END_SEQUENCE;
         }

src/credentials/GroupDataProviderImpl.cpp

@@ -993,7 +993,7 @@ CHIP_ERROR GroupDataProviderImpl::RemoveGroupInfoAt(chip::FabricIndex fabric_ind
         {
             break;
         }
-        endpoint.Delete(mStorage);
+        TEMPORARY_RETURN_IGNORED endpoint.Delete(mStorage);
         endpoint.endpoint_id = endpoint.next;
     }
 
@@ -1105,7 +1105,7 @@ CHIP_ERROR GroupDataProviderImpl::RemoveEndpoint(chip::FabricIndex fabric_index,
     VerifyOrReturnError(endpoint.Find(mStorage, fabric, group, endpoint_id), CHIP_ERROR_NOT_FOUND);
 
     // Existing endpoint
-    endpoint.Delete(mStorage);
+    TEMPORARY_RETURN_IGNORED endpoint.Delete(mStorage);
 
     if (endpoint.first)
     {
@@ -1331,7 +1331,7 @@ CHIP_ERROR GroupDataProviderImpl::RemoveEndpoints(chip::FabricIndex fabric_index
     while (endpoint_index < group.endpoint_count)
     {
         ReturnErrorOnFailure(endpoint.Load(mStorage));
-        endpoint.Delete(mStorage);
+        TEMPORARY_RETURN_IGNORED endpoint.Delete(mStorage);
         endpoint.endpoint_id = endpoint.next;
         endpoint_index++;
     }
@@ -1459,7 +1459,7 @@ CHIP_ERROR GroupDataProviderImpl::RemoveGroupKeys(chip::FabricIndex fabric_index
         {
             break;
         }
-        map.Delete(mStorage);
+        TEMPORARY_RETURN_IGNORED map.Delete(mStorage);
         map.id = map.next;
     }
 
@@ -1627,7 +1627,7 @@ CHIP_ERROR GroupDataProviderImpl::RemoveKeySet(chip::FabricIndex fabric_index, u
     uint16_t original_count = fabric.map_count;
     for (uint16_t i = 0; i < original_count; ++i)
     {
-        fabric.Load(mStorage);
+        TEMPORARY_RETURN_IGNORED fabric.Load(mStorage);
         size_t idx = map.Find(mStorage, fabric, target_id);
         if (idx == std::numeric_limits<size_t>::max())
         {
@@ -1636,7 +1636,7 @@ CHIP_ERROR GroupDataProviderImpl::RemoveKeySet(chip::FabricIndex fabric_index, u
         // NOTE: It's unclear what should happen here if we have removed the key set
         // and possibly some mappings before failing. For now, ignoring errors, but
         // open to suggestsions for the correct behavior.
-        RemoveGroupKeyAt(fabric_index, idx);
+        TEMPORARY_RETURN_IGNORED RemoveGroupKeyAt(fabric_index, idx);
     }
     return CHIP_NO_ERROR;
 }
@@ -1706,14 +1706,14 @@ CHIP_ERROR GroupDataProviderImpl::RemoveFabric(chip::FabricIndex fabric_index)
 
     for (size_t i = 0; i < fabric.map_count; i++)
     {
-        RemoveGroupKeyAt(fabric_index, fabric.map_count - i - 1);
+        TEMPORARY_RETURN_IGNORED RemoveGroupKeyAt(fabric_index, fabric.map_count - i - 1);
     }
 
     // Remove group info
 
     for (size_t i = 0; i < fabric.group_count; i++)
     {
-        RemoveGroupInfoAt(fabric_index, fabric.group_count - i - 1);
+        TEMPORARY_RETURN_IGNORED RemoveGroupInfoAt(fabric_index, fabric.group_count - i - 1);
     }
 
     // Remove Keysets
@@ -1728,7 +1728,7 @@ CHIP_ERROR GroupDataProviderImpl::RemoveFabric(chip::FabricIndex fabric_index)
         {
             break;
         }
-        RemoveKeySet(fabric_index, keyset.keyset_id);
+        TEMPORARY_RETURN_IGNORED RemoveKeySet(fabric_index, keyset.keyset_id);
         keyset.keyset_id = keyset.next;
         keyset_count++;
     }
@@ -1926,7 +1926,7 @@ bool GroupDataProviderImpl::GroupSessionIteratorImpl::Next(GroupSession & output
         KeySetData keyset;
         VerifyOrReturnError(keyset.Find(mProvider.mStorage, fabric, mapping.keyset_id), false);
 
-        if (mKeyIndex >= keyset.keys_count)
+        if (mKeyIndex >= keyset.keys_count || (mKeyIndex >= KeySet::kEpochKeysMax))
         {
             // No more keys in current keyset, try next
             mMapping = mapping.next;
@@ -1938,7 +1938,7 @@ bool GroupDataProviderImpl::GroupSessionIteratorImpl::Next(GroupSession & output
         Crypto::GroupOperationalCredentials & creds = keyset.operational_keys[mKeyIndex++];
         if (creds.hash == mSessionId)
         {
-            mGroupKeyContext.Initialize(creds.encryption_key, mSessionId, creds.privacy_key);
+            TEMPORARY_RETURN_IGNORED mGroupKeyContext.Initialize(creds.encryption_key, mSessionId, creds.privacy_key);
             output.fabric_index    = fabric.fabric_index;
             output.group_id        = mapping.group_id;
             output.security_policy = keyset.policy;

src/credentials/GroupDataProviderImpl.h

@@ -161,11 +161,11 @@ class GroupDataProviderImpl : public GroupDataProvider
             mProvider(provider)
 
         {
-            Initialize(encryptionKey, hash, privacyKey);
+            TEMPORARY_RETURN_IGNORED Initialize(encryptionKey, hash, privacyKey);
         }
 
-        void Initialize(const Crypto::Symmetric128BitsKeyByteArray & encryptionKey, uint16_t hash,
-                        const Crypto::Symmetric128BitsKeyByteArray & privacyKey)
+        CHIP_ERROR Initialize(const Crypto::Symmetric128BitsKeyByteArray & encryptionKey, uint16_t hash,
+                              const Crypto::Symmetric128BitsKeyByteArray & privacyKey)
         {
             ReleaseKeys();
             mKeyHash = hash;
@@ -176,8 +176,8 @@ class GroupDataProviderImpl : public GroupDataProvider
             // like more work, so let's use the transitional code below for now.
 
             Crypto::SessionKeystore * keystore = mProvider.GetSessionKeystore();
-            keystore->CreateKey(encryptionKey, mEncryptionKey);
-            keystore->CreateKey(privacyKey, mPrivacyKey);
+            ReturnErrorOnFailure(keystore->CreateKey(encryptionKey, mEncryptionKey));
+            return keystore->CreateKey(privacyKey, mPrivacyKey);
         }
 
         void ReleaseKeys()