Address review comments.

project-chip · Mar 23, 2022 · 52bfb96 · 52bfb96
1 parent 907717f
commit 52bfb96
Show file tree

Hide file tree

Showing 2 changed files with 80 additions and 63 deletions.
diff --git a/src/credentials/FabricTable.cpp b/src/credentials/FabricTable.cpp
@@ -764,7 +764,10 @@ CHIP_ERROR FabricTable::Delete(FabricIndex index)
 
     if (!mNextAvailableFabricIndex.HasValue())
     {
-        // We have one available now.
+        // We must have been in a situation where CHIP_CONFIG_MAX_FABRICS is 254
+        // and our fabric table was full, so there was no valid next index.  We
+        // have a single available index now, though; use it as
+        // mNextAvailableFabricIndex.
         mNextAvailableFabricIndex.SetValue(index);
     }
     // If StoreFabricIndexInfo fails here, that's probably OK.  When we try to
@@ -836,68 +839,7 @@ CHIP_ERROR FabricTable::Init(PersistentStorageDelegate * storage)
         TLV::ContiguousBufferTLVReader reader;
         reader.Init(buf, size);
 
-        ReturnErrorOnFailure(reader.Next(TLV::kTLVType_Structure, TLV::AnonymousTag()));
-        TLV::TLVType containerType;
-        ReturnErrorOnFailure(reader.EnterContainer(containerType));
-
-        ReturnErrorOnFailure(reader.Next(kNextAvailableFabricIndexTag));
-        if (reader.GetType() == TLV::kTLVType_Null)
-        {
-            mNextAvailableFabricIndex.ClearValue();
-        }
-        else
-        {
-            ReturnErrorOnFailure(reader.Get(mNextAvailableFabricIndex.Emplace()));
-        }
-
-        ReturnErrorOnFailure(reader.Next(TLV::kTLVType_Array, kFabricIndicesTag));
-        TLV::TLVType arrayType;
-        ReturnErrorOnFailure(reader.EnterContainer(arrayType));
-
-        while ((err = reader.Next()) == CHIP_NO_ERROR)
-        {
-            if (mFabricCount >= ArraySize(mStates))
-            {
-                // We have nowhere to deserialize this fabric info into.
-                return CHIP_ERROR_NO_MEMORY;
-            }
-
-            auto & fabric = mStates[mFabricCount];
-            ReturnErrorOnFailure(reader.Get(fabric.mFabricIndex));
-
-            err = LoadFromStorage(&fabric);
-            if (err == CHIP_NO_ERROR)
-            {
-                ++mFabricCount;
-            }
-            else
-            {
-                // This could happen if we failed to store our fabric index info
-                // after we deleted the fabric from storage.  Just ignore this
-                // fabric index and keep going.
-            }
-        }
-
-        if (err != CHIP_END_OF_TLV)
-        {
-            return err;
-        }
-
-        ReturnErrorOnFailure(reader.ExitContainer(arrayType));
-        ReturnErrorOnFailure(reader.ExitContainer(containerType));
-        ReturnErrorOnFailure(reader.VerifyEndOfContainer());
-
-        if (!mNextAvailableFabricIndex.HasValue() && mFabricCount < kMaxValidFabricIndex)
-        {
-            // We must have a fabric index available here. This situation could
-            // happen if we fail to store fabric index info when deleting a
-            // fabric.
-            mNextAvailableFabricIndex.SetValue(kMinValidFabricIndex);
-            if (FindFabricWithIndex(kMinValidFabricIndex))
-            {
-                UpdateNextAvailableFabricIndex();
-            }
-        }
+        ReturnErrorOnFailure(ReadFabricInfo(reader));
     }
 
     return CHIP_NO_ERROR;
@@ -988,6 +930,75 @@ CHIP_ERROR FabricTable::StoreFabricIndexInfo() const
     return CHIP_NO_ERROR;
 }
 
+CHIP_ERROR FabricTable::ReadFabricInfo(TLV::ContiguousBufferTLVReader & reader)
+{
+    ReturnErrorOnFailure(reader.Next(TLV::kTLVType_Structure, TLV::AnonymousTag()));
+    TLV::TLVType containerType;
+    ReturnErrorOnFailure(reader.EnterContainer(containerType));
+
+    ReturnErrorOnFailure(reader.Next(kNextAvailableFabricIndexTag));
+    if (reader.GetType() == TLV::kTLVType_Null)
+    {
+        mNextAvailableFabricIndex.ClearValue();
+    }
+    else
+    {
+        ReturnErrorOnFailure(reader.Get(mNextAvailableFabricIndex.Emplace()));
+    }
+
+    ReturnErrorOnFailure(reader.Next(TLV::kTLVType_Array, kFabricIndicesTag));
+    TLV::TLVType arrayType;
+    ReturnErrorOnFailure(reader.EnterContainer(arrayType));
+
+    CHIP_ERROR err;
+    while ((err = reader.Next()) == CHIP_NO_ERROR)
+    {
+        if (mFabricCount >= ArraySize(mStates))
+        {
+            // We have nowhere to deserialize this fabric info into.
+            return CHIP_ERROR_NO_MEMORY;
+        }
+
+        auto & fabric = mStates[mFabricCount];
+        ReturnErrorOnFailure(reader.Get(fabric.mFabricIndex));
+
+        err = LoadFromStorage(&fabric);
+        if (err == CHIP_NO_ERROR)
+        {
+            ++mFabricCount;
+        }
+        else
+        {
+            // This could happen if we failed to store our fabric index info
+            // after we deleted the fabric from storage.  Just ignore this
+            // fabric index and keep going.
+        }
+    }
+
+    if (err != CHIP_END_OF_TLV)
+    {
+        return err;
+    }
+
+    ReturnErrorOnFailure(reader.ExitContainer(arrayType));
+    ReturnErrorOnFailure(reader.ExitContainer(containerType));
+    ReturnErrorOnFailure(reader.VerifyEndOfContainer());
+
+    if (!mNextAvailableFabricIndex.HasValue() && mFabricCount < kMaxValidFabricIndex)
+    {
+        // We must have a fabric index available here. This situation could
+        // happen if we fail to store fabric index info when deleting a
+        // fabric.
+        mNextAvailableFabricIndex.SetValue(kMinValidFabricIndex);
+        if (FindFabricWithIndex(kMinValidFabricIndex))
+        {
+            UpdateNextAvailableFabricIndex();
+        }
+    }
+
+    return CHIP_NO_ERROR;
+}
+
 CHIP_ERROR FabricInfo::TestOnlyBuildFabric(ByteSpan rootCert, ByteSpan icacCert, ByteSpan nocCert, ByteSpan nodePubKey,
                                            ByteSpan nodePrivateKey)
 {

diff --git a/src/credentials/FabricTable.h b/src/credentials/FabricTable.h
@@ -438,6 +438,12 @@ class DLL_EXPORT FabricTable
      */
     CHIP_ERROR StoreFabricIndexInfo() const;
 
+    /**
+     * Read our fabric index info from the given TLV reader and set up the
+     * fabric table accordingly.
+     */
+    CHIP_ERROR ReadFabricInfo(TLV::ContiguousBufferTLVReader & reader);
+
     FabricInfo mStates[CHIP_CONFIG_MAX_FABRICS];
     PersistentStorageDelegate * mStorage = nullptr;