Pass the correct TokenNumber into a CallbackOnSet notification function. The value being passed in was off by 1.

This would not normally cause an issue because the correct notification function was being called.  There is only an issue if the same notification function is being used for multiple tokens, and the notification function needs to use the TokenNumber parameter to determine which token setting caused the callback function to be called.

Update the logic so the CallbackOnSet notification functions are not called with the internal PCD database lock in the acquired state.  This allows other PCD Get/Set operations to be performed from within a CallbackOnSet notification.

Update logic so the PCD database lock is not left in the acquired state when SetWorker() returns EFI_INVALID_PARAMETER.  


git-svn-id: https://edk2.svn.sourceforge.net/svnroot/edk2/trunk/edk2@9981 6f19259b-4bc3-4df7-8a09-765794883524

1 files changed, 22 insertions, 28 deletions

diff --git a/MdeModulePkg/Universal/PCD/Dxe/Service.c b/MdeModulePkg/Universal/PCD/Dxe/Service.c
index 5cd9eb8ce5..59038fb452 100644
--- a/MdeModulePkg/Universal/PCD/Dxe/Service.c
+++ b/MdeModulePkg/Universal/PCD/Dxe/Service.c
@@ -14,9 +14,9 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 

 #include "Service.h"

 

-PCD_DATABASE * mPcdDatabase;

+PCD_DATABASE  *mPcdDatabase;

 

-LIST_ENTRY *mCallbackFnTable;

+LIST_ENTRY    *mCallbackFnTable;

 

 /**

   Get the PCD entry pointer in PCD database.

@@ -213,11 +213,9 @@ DxeRegisterCallBackWorker (
   //

   // TokenNumber Zero is reserved as PCD_INVALID_TOKEN_NUMBER.

   // We have to decrement TokenNumber by 1 to make it usable

-  // as the array index.

+  // as the array index of mCallbackFnTable[].

   //

-  TokenNumber--;

-

-  ListHead = &mCallbackFnTable[TokenNumber];

+  ListHead = &mCallbackFnTable[TokenNumber - 1];

   ListNode = GetFirstNode (ListHead);

 

   while (ListNode != ListHead) {

@@ -274,11 +272,9 @@ DxeUnRegisterCallBackWorker (
   //

   // TokenNumber Zero is reserved as PCD_INVALID_TOKEN_NUMBER.

   // We have to decrement TokenNumber by 1 to make it usable

-  // as the array index.

+  // as the array index of mCallbackFnTable[].

   //

-  TokenNumber--;

-

-  ListHead = &mCallbackFnTable[TokenNumber];

+  ListHead = &mCallbackFnTable[TokenNumber - 1];

   ListNode = GetFirstNode (ListHead);

 

   while (ListNode != ListHead) {

@@ -624,15 +620,13 @@ InvokeCallbackOnSet (
   //

   // TokenNumber Zero is reserved as PCD_INVALID_TOKEN_NUMBER.

   // We have to decrement TokenNumber by 1 to make it usable

-  // as the array index.

+  // as the array index of mCallbackFnTable[].

   //

-  TokenNumber--;

-  

-  ListHead = &mCallbackFnTable[TokenNumber];

+  ListHead = &mCallbackFnTable[TokenNumber - 1];

   ListNode = GetFirstNode (ListHead);

 

   while (ListNode != ListHead) {

-    FnTableEntry = CR_FNENTRY_FROM_LISTNODE(ListNode, CALLBACK_FN_ENTRY, Node);

+    FnTableEntry = CR_FNENTRY_FROM_LISTNODE (ListNode, CALLBACK_FN_ENTRY, Node);

 

     FnTableEntry->CallbackFn(Guid, 

                     (Guid == NULL) ? TokenNumber : ExTokenNumber,

@@ -708,11 +702,6 @@ SetWorker (
   UINTN               TmpTokenNumber;

 

   //

-  // Aquire lock to prevent reentrance from TPL_CALLBACK level

-  //

-  EfiAcquireLock (&mPcdDatabaseLock);

-

-  //

   // TokenNumber Zero is reserved as PCD_INVALID_TOKEN_NUMBER.

   // We have to decrement TokenNumber by 1 to make it usable

   // as the array index.

@@ -731,26 +720,31 @@ SetWorker (
   if ((!PtrType) && (*Size != DxePcdGetSize (TokenNumber + 1))) {

     return EFI_INVALID_PARAMETER;

   }

-  

+

   //

   // EBC compiler is very choosy. It may report warning about comparison

   // between UINTN and 0 . So we add 1 in each size of the 

   // comparison.

   //

-  IsPeiDb = (BOOLEAN) ((TokenNumber + 1 < PEI_LOCAL_TOKEN_NUMBER + 1) ? TRUE : FALSE);

+  if ((TokenNumber + 1 < PEI_NEX_TOKEN_NUMBER + 1) ||

+      (TokenNumber + 1 >= PEI_LOCAL_TOKEN_NUMBER + 1 || TokenNumber + 1 < (PEI_LOCAL_TOKEN_NUMBER + DXE_NEX_TOKEN_NUMBER + 1))) {

+    InvokeCallbackOnSet (0, NULL, TokenNumber + 1, Data, *Size);

+  }

 

-  LocalTokenNumberTable  = IsPeiDb ? mPcdDatabase->PeiDb.Init.LocalTokenNumberTable : 

-                                     mPcdDatabase->DxeDb.Init.LocalTokenNumberTable;

+  //

+  // Aquire lock to prevent reentrance from TPL_CALLBACK level

+  //

+  EfiAcquireLock (&mPcdDatabaseLock);

 

   //

   // EBC compiler is very choosy. It may report warning about comparison

   // between UINTN and 0 . So we add 1 in each size of the 

   // comparison.

   //

-  if ((TokenNumber + 1 < PEI_NEX_TOKEN_NUMBER + 1) ||

-      (TokenNumber + 1 >= PEI_LOCAL_TOKEN_NUMBER + 1 || TokenNumber + 1 < (PEI_LOCAL_TOKEN_NUMBER + DXE_NEX_TOKEN_NUMBER + 1))) {

-    InvokeCallbackOnSet (0, NULL, TokenNumber + 1, Data, *Size);

-  }

+  IsPeiDb = (BOOLEAN) ((TokenNumber + 1 < PEI_LOCAL_TOKEN_NUMBER + 1) ? TRUE : FALSE);

+

+  LocalTokenNumberTable  = IsPeiDb ? mPcdDatabase->PeiDb.Init.LocalTokenNumberTable : 

+                                     mPcdDatabase->DxeDb.Init.LocalTokenNumberTable;

 

   TokenNumber = IsPeiDb ? TokenNumber

                         : TokenNumber - PEI_LOCAL_TOKEN_NUMBER;