Fetch next occupied credential index earlier in getCredentialStatusCo…

…mmandHandler. We want do to that before we actually fetch the credential we are trying to get the status of, since determining the next occupied credential index might stomp on data returned by emberAfPluginDoorLockGetCredential.
bzbarsky-apple · Oct 2, 2024 · 92c6b1a · 92c6b1a
1 parent 83159c2
commit 92c6b1a
Show file tree

Hide file tree

Showing 2 changed files with 28 additions and 12 deletions.
diff --git a/src/app/clusters/door-lock-server/door-lock-server.cpp b/src/app/clusters/door-lock-server/door-lock-server.cpp
@@ -860,10 +860,27 @@ void DoorLockServer::getCredentialStatusCommandHandler(chip::app::CommandHandler
         return;
     }
 
+    // Our response will need to include the index of the next occupied credential slot
+    // after credentialIndex, if there is one.
+    //
+    // We want to figure this out before we call emberAfPluginDoorLockGetCredential, because to do
+    // so we will also need to call emberAfPluginDoorLockGetCredential, and the
+    // EmberAfPluginDoorLockCredentialInfo we get might be pointing into some application-static
+    // buffers (for its credential data and whatnot).
+    DataModel::Nullable<uint16_t> nextCredentialIndex;
+    {
+        uint16_t foundNextCredentialIndex;
+        if (findOccupiedCredentialSlot(commandPath.mEndpointId, credentialType, static_cast<uint16_t>(credentialIndex + 1),
+                                       foundNextCredentialIndex))
+        {
+            nextCredentialIndex.SetNonNull(foundNextCredentialIndex);
+        }
+    }
+
     uint16_t maxNumberOfCredentials = 0;
     if (!credentialIndexValid(commandPath.mEndpointId, credentialType, credentialIndex, maxNumberOfCredentials))
     {
-        sendGetCredentialResponse(commandObj, commandPath, credentialType, credentialIndex, 0, nullptr, false);
+        sendGetCredentialResponse(commandObj, commandPath, credentialType, credentialIndex, nextCredentialIndex, 0, nullptr, false);
         return;
     }
 
@@ -896,8 +913,8 @@ void DoorLockServer::getCredentialStatusCommandHandler(chip::app::CommandHandler
         }
     }
 
-    sendGetCredentialResponse(commandObj, commandPath, credentialType, credentialIndex, userIndexWithCredential, &credentialInfo,
-                              credentialExists);
+    sendGetCredentialResponse(commandObj, commandPath, credentialType, credentialIndex, nextCredentialIndex,
+                              userIndexWithCredential, &credentialInfo, credentialExists);
 }
 
 namespace {
@@ -918,9 +935,12 @@ bool IsAliroCredentialType(CredentialTypeEnum credentialType)
 void DoorLockServer::sendGetCredentialResponse(chip::app::CommandHandler * commandObj,
                                                const chip::app::ConcreteCommandPath & commandPath,
                                                CredentialTypeEnum credentialType, uint16_t credentialIndex,
-                                               uint16_t userIndexWithCredential,
+                                               DataModel::Nullable<uint16_t> nextCredentialIndex, uint16_t userIndexWithCredential,
                                                EmberAfPluginDoorLockCredentialInfo * credentialInfo, bool credentialExists)
 {
+    // Important: We have to make sure nothing in this function calls
+    // emberAfPluginDoorLockGetCredential, because that might stomp on the data
+    // pointed to by credentialInfo.
     Commands::GetCredentialStatusResponse::Type response{ .credentialExists = credentialExists };
     if (credentialExists && !(nullptr == credentialInfo))
     {
@@ -949,19 +969,14 @@ void DoorLockServer::sendGetCredentialResponse(chip::app::CommandHandler * comma
             response.credentialData.Emplace(NullNullable);
         }
     }
-    uint16_t nextCredentialIndex = 0;
-    if (findOccupiedCredentialSlot(commandPath.mEndpointId, credentialType, static_cast<uint16_t>(credentialIndex + 1),
-                                   nextCredentialIndex))
-    {
-        response.nextCredentialIndex.SetNonNull(nextCredentialIndex);
-    }
+    response.nextCredentialIndex = nextCredentialIndex;
     commandObj->AddResponse(commandPath, response);
 
     ChipLogProgress(Zcl,
                     "[GetCredentialStatus] Prepared credential status "
                     "[endpointId=%d,credentialType=%u,credentialIndex=%d,userIndex=%d,nextCredentialIndex=%d]",
                     commandPath.mEndpointId, to_underlying(credentialType), credentialIndex, userIndexWithCredential,
-                    nextCredentialIndex);
+                    nextCredentialIndex.ValueOr(0));
 }
 
 void DoorLockServer::clearCredentialCommandHandler(

diff --git a/src/app/clusters/door-lock-server/door-lock-server.h b/src/app/clusters/door-lock-server/door-lock-server.h
@@ -449,7 +449,8 @@ class DoorLockServer : public chip::app::AttributeAccessInterface
                                            uint16_t credentialIndex);
 
     void sendGetCredentialResponse(chip::app::CommandHandler * commandObj, const chip::app::ConcreteCommandPath & commandPath,
-                                   CredentialTypeEnum credentialType, uint16_t credentialIndex, uint16_t userIndexWithCredential,
+                                   CredentialTypeEnum credentialType, uint16_t credentialIndex,
+                                   chip::app::DataModel::Nullable<uint16_t> nextCredentialIndex, uint16_t userIndexWithCredential,
                                    EmberAfPluginDoorLockCredentialInfo * credentialInfo, bool credentialExists);
 
     void clearCredentialCommandHandler(chip::app::CommandHandler * commandObj, const chip::app::ConcreteCommandPath & commandPath,