Enhance the check for options in the question.

Signed-off-by: Eric Dong <eric.dong@intel.com>
Reviewed-by: Liming Gao <liming.gao@intel.com>


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

4 files changed, 174 insertions, 102 deletions

diff --git a/MdeModulePkg/Universal/SetupBrowserDxe/InputHandler.c b/MdeModulePkg/Universal/SetupBrowserDxe/InputHandler.c
index 81190464cb..720efb9a04 100644
--- a/MdeModulePkg/Universal/SetupBrowserDxe/InputHandler.c
+++ b/MdeModulePkg/Universal/SetupBrowserDxe/InputHandler.c
@@ -987,48 +987,49 @@ GetSelectionInputPopUp (
   }

 

   //

-  // Prepare HiiValue array

+  // Move valid Option to list head.

   //

-  HiiValueArray = AllocateZeroPool (OptionCount * sizeof (EFI_HII_VALUE));

-  ASSERT (HiiValueArray != NULL);

-  Link = GetFirstNode (&Question->OptionListHead);

-  for (Index = 0; Index < OptionCount; Index++) {

-    if (OrderedList) {

+  PopUpMenuLines = 0;

+  if (OrderedList) {

+    //

+    // Prepare HiiValue array

+    //  

+    HiiValueArray = AllocateZeroPool (OptionCount * sizeof (EFI_HII_VALUE));

+    ASSERT (HiiValueArray != NULL);

+    for (Index = 0; Index < OptionCount; Index++) {

       HiiValueArray[Index].Type = ValueType;

       HiiValueArray[Index].Value.u64 = GetArrayData (ValueArray, ValueType, Index);

-    } else {

-      OneOfOption = QUESTION_OPTION_FROM_LINK (Link);

-      CopyMem (&HiiValueArray[Index], &OneOfOption->Value, sizeof (EFI_HII_VALUE));

-      Link = GetNextNode (&Question->OptionListHead, Link);

     }

-  }

 

-  //

-  // Move Suppressed Option to list tail

-  //

-  PopUpMenuLines = 0;

-  for (Index = 0; Index < OptionCount; Index++) {

-    OneOfOption = ValueToOption (Question, &HiiValueArray[OptionCount - Index - 1]);

-    if (OneOfOption == NULL) {

-      return EFI_NOT_FOUND;

-    }

+    for (Index = 0; Index < OptionCount; Index++) {

+      OneOfOption = ValueToOption (Question, &HiiValueArray[OptionCount - Index - 1]);

+      if (OneOfOption == NULL) {

+        return EFI_NOT_FOUND;

+      }

 

-    RemoveEntryList (&OneOfOption->Link);

+      RemoveEntryList (&OneOfOption->Link);

 

-    if ((OneOfOption->SuppressExpression != NULL) &&

-        EvaluateExpressionList(OneOfOption->SuppressExpression, FALSE, NULL, NULL) != ExpressFalse) {

-      //

-      // This option is suppressed, insert to tail

       //

-      InsertTailList (&Question->OptionListHead, &OneOfOption->Link);

-    } else {

-      //

-      // Insert to head

+      // Insert to head.

       //

       InsertHeadList (&Question->OptionListHead, &OneOfOption->Link);

 

       PopUpMenuLines++;

     }

+

+    FreePool (HiiValueArray);

+  } else {

+    Link = GetFirstNode (&Question->OptionListHead);

+    for (Index = 0; Index < OptionCount; Index++) {

+      OneOfOption = QUESTION_OPTION_FROM_LINK (Link);

+      Link = GetNextNode (&Question->OptionListHead, Link);

+      if ((OneOfOption->SuppressExpression == NULL) ||

+            EvaluateExpressionList(OneOfOption->SuppressExpression, FALSE, NULL, NULL) == ExpressFalse) {

+        RemoveEntryList (&OneOfOption->Link);

+        InsertHeadList (&Question->OptionListHead, &OneOfOption->Link);

+        PopUpMenuLines++;

+      }

+    }

   }

 

   //

@@ -1310,7 +1311,6 @@ TheKey:
           }

         }

 

-        FreePool (HiiValueArray);

         return EFI_DEVICE_ERROR;

 

       default:

@@ -1328,6 +1328,12 @@ TheKey:
         Link = GetFirstNode (&Question->OptionListHead);

         while (!IsNull (&Question->OptionListHead, Link)) {

           OneOfOption = QUESTION_OPTION_FROM_LINK (Link);

+          Link = GetNextNode (&Question->OptionListHead, Link);

+

+          if ((OneOfOption->SuppressExpression != NULL) &&

+              EvaluateExpressionList(OneOfOption->SuppressExpression, FALSE, NULL, NULL) != ExpressFalse) {

+            continue;

+          }

 

           SetArrayData (ValueArray, ValueType, Index, OneOfOption->Value.Value.u64);

 

@@ -1335,8 +1341,6 @@ TheKey:
           if (Index > Question->MaxContainers) {

             break;

           }

-

-          Link = GetNextNode (&Question->OptionListHead, Link);

         }

       } else {

         ASSERT (CurrentOption != NULL);

@@ -1344,7 +1348,6 @@ TheKey:
       }

 

       gST->ConOut->SetAttribute (gST->ConOut, SavedAttribute);

-      FreePool (HiiValueArray);

 

       Status = ValidateQuestion (Selection->FormSet, Selection->Form, Question, EFI_HII_EXPRESSION_INCONSISTENT_IF);

       if (EFI_ERROR (Status)) {

diff --git a/MdeModulePkg/Universal/SetupBrowserDxe/ProcessOptions.c b/MdeModulePkg/Universal/SetupBrowserDxe/ProcessOptions.c
index 181afec16d..fecb98e8bf 100644
--- a/MdeModulePkg/Universal/SetupBrowserDxe/ProcessOptions.c
+++ b/MdeModulePkg/Universal/SetupBrowserDxe/ProcessOptions.c
@@ -91,7 +91,13 @@ ValueToOption (
     Option = QUESTION_OPTION_FROM_LINK (Link);

 

     if ((CompareHiiValue (&Option->Value, OptionValue, &Result, NULL) == EFI_SUCCESS) && (Result == 0)) {

-      return Option;

+      //

+      // Check the suppressif condition, only a valid option can be return.

+      //

+      if ((Option->SuppressExpression == NULL) ||

+          ((EvaluateExpressionList(Option->SuppressExpression, FALSE, NULL, NULL) == ExpressFalse))) {

+        return Option;

+      }

     }

 

     Link = GetNextNode (&Question->OptionListHead, Link);

@@ -190,6 +196,47 @@ SetArrayData (
   }

 }

 

+/**

+  Check whether this value already in the array, if yes, return the index.

+

+  @param  Array                  The data array.

+  @param  Type                   Type of the data in this array.

+  @param  Value                  The value to be find.

+  @param  Index                  The index in the array which has same value with Value.

+  

+  @retval   TRUE Found the value in the array.

+  @retval   FALSE Not found the value.

+

+**/

+BOOLEAN 

+FindArrayData (

+  IN VOID                     *Array,

+  IN UINT8                    Type,

+  IN UINT64                   Value,

+  OUT UINTN                   *Index OPTIONAL

+  )

+{

+  UINTN  Count;

+  UINT64 TmpValue;

+  

+  ASSERT (Array != NULL);

+

+  Count    = 0;

+  TmpValue = 0;

+

+  while ((TmpValue = GetArrayData (Array, Type, Count)) != 0) {

+    if (Value == TmpValue) {

+      if (Index != NULL) {

+        *Index = Count;

+      }

+      return TRUE;

+    }

+

+    Count ++;

+  }

+

+  return FALSE;

+}

 

 /**

   Print Question Value according to it's storage width and display attributes.

@@ -396,7 +443,6 @@ ProcessOptions (
   LIST_ENTRY                      *Link;

   EFI_HII_VALUE                   HiiValue;

   EFI_HII_VALUE                   *QuestionValue;

-  BOOLEAN                         Suppress;

   UINT16                          Maximum;

   QUESTION_OPTION                 *Option;

   UINTN                           Index2;

@@ -476,9 +522,13 @@ ProcessOptions (
           Index2 = 0;

           while (!IsNull (&Question->OptionListHead, Link) && Index2 < Question->MaxContainers) {

             Option = QUESTION_OPTION_FROM_LINK (Link);

+            Link = GetNextNode (&Question->OptionListHead, Link);

+            if ((Option->SuppressExpression != NULL) &&

+                ((EvaluateExpressionList(Option->SuppressExpression, FALSE, NULL, NULL) == ExpressSuppress))) {

+              continue;

+            }

             SetArrayData (ValueArray, ValueType, Index2, Option->Value.Value.u64);

             Index2++;

-            Link = GetNextNode (&Question->OptionListHead, Link);

           }

           SetArrayData (ValueArray, ValueType, Index2, 0);

 

@@ -490,28 +540,46 @@ ProcessOptions (
           return EFI_NOT_FOUND;

         }

 

-        Suppress = FALSE;

+        Character[0] = LEFT_ONEOF_DELIMITER;

+        NewStrCat (OptionString[0], Character);

+        StringPtr = GetToken (OneOfOption->Text, Selection->Handle);

+        ASSERT (StringPtr != NULL);

+        NewStrCat (OptionString[0], StringPtr);

+        Character[0] = RIGHT_ONEOF_DELIMITER;

+        NewStrCat (OptionString[0], Character);

+        Character[0] = CHAR_CARRIAGE_RETURN;

+        NewStrCat (OptionString[0], Character);

+        FreePool (StringPtr);

+      }

+

+      //

+      // Search the other options, try to find the one not in the container.

+      //

+      Link = GetFirstNode (&Question->OptionListHead);

+      while (!IsNull (&Question->OptionListHead, Link)) {

+        OneOfOption = QUESTION_OPTION_FROM_LINK (Link);

+        Link = GetNextNode (&Question->OptionListHead, Link);

         if ((OneOfOption->SuppressExpression != NULL) &&

-            (EvaluateExpressionList(OneOfOption->SuppressExpression, FALSE, NULL, NULL) == ExpressSuppress)) {

-          //

-          // This option is suppressed

-          //

-          Suppress = TRUE;

+            ((EvaluateExpressionList(OneOfOption->SuppressExpression, FALSE, NULL, NULL) == ExpressSuppress))) {

+          continue;

         }

 

-        if (!Suppress) {

-          Character[0] = LEFT_ONEOF_DELIMITER;

-          NewStrCat (OptionString[0], Character);

-          StringPtr = GetToken (OneOfOption->Text, Selection->Handle);

-          ASSERT (StringPtr != NULL);

-          NewStrCat (OptionString[0], StringPtr);

-          Character[0] = RIGHT_ONEOF_DELIMITER;

-          NewStrCat (OptionString[0], Character);

-          Character[0] = CHAR_CARRIAGE_RETURN;

-          NewStrCat (OptionString[0], Character);

-

-          FreePool (StringPtr);

+        if (FindArrayData (ValueArray, ValueType, OneOfOption->Value.Value.u64, NULL)) {

+          continue;

         }

+

+        SetArrayData (ValueArray, ValueType, Index++, OneOfOption->Value.Value.u64);

+

+        Character[0] = LEFT_ONEOF_DELIMITER;

+        NewStrCat (OptionString[0], Character);

+        StringPtr = GetToken (OneOfOption->Text, Selection->Handle);

+        ASSERT (StringPtr != NULL);

+        NewStrCat (OptionString[0], StringPtr);

+        Character[0] = RIGHT_ONEOF_DELIMITER;

+        NewStrCat (OptionString[0], Character);

+        Character[0] = CHAR_CARRIAGE_RETURN;

+        NewStrCat (OptionString[0], Character);

+        FreePool (StringPtr);

       }

     }

     break;

@@ -564,50 +632,15 @@ ProcessOptions (
         return EFI_NOT_FOUND;

       }

 

-      if ((OneOfOption->SuppressExpression != NULL) &&

-          ((EvaluateExpressionList(OneOfOption->SuppressExpression, FALSE, NULL, NULL) == ExpressSuppress))) {

-        //

-        // This option is suppressed

-        //

-        Suppress = TRUE;

-      } else {

-        Suppress = FALSE;

-      }

-

-      if (Suppress) {

-        //

-        // Current selected option happen to be suppressed,

-        // enforce to select on a non-suppressed option

-        //

-        Link = GetFirstNode (&Question->OptionListHead);

-        while (!IsNull (&Question->OptionListHead, Link)) {

-          OneOfOption = QUESTION_OPTION_FROM_LINK (Link);

+      Character[0] = LEFT_ONEOF_DELIMITER;

+      NewStrCat (OptionString[0], Character);

+      StringPtr = GetToken (OneOfOption->Text, Selection->Handle);

+      ASSERT (StringPtr != NULL);

+      NewStrCat (OptionString[0], StringPtr);

+      Character[0] = RIGHT_ONEOF_DELIMITER;

+      NewStrCat (OptionString[0], Character);

 

-          if ((OneOfOption->SuppressExpression == NULL) ||

-              (EvaluateExpressionList(OneOfOption->SuppressExpression, FALSE, NULL, NULL) == ExpressFalse)) {

-            Suppress = FALSE;

-            CopyMem (QuestionValue, &OneOfOption->Value, sizeof (EFI_HII_VALUE));

-            SetQuestionValue (Selection->FormSet, Selection->Form, Question, GetSetValueWithEditBuffer);

-            UpdateStatusBar (Selection, NV_UPDATE_REQUIRED, Question->QuestionFlags, TRUE);

-            gST->ConOut->SetAttribute (gST->ConOut, PcdGet8 (PcdBrowserFieldTextColor) | FIELD_BACKGROUND);

-            break;

-          }

-

-          Link = GetNextNode (&Question->OptionListHead, Link);

-        }

-      }

-

-      if (!Suppress) {

-        Character[0] = LEFT_ONEOF_DELIMITER;

-        NewStrCat (OptionString[0], Character);

-        StringPtr = GetToken (OneOfOption->Text, Selection->Handle);

-        ASSERT (StringPtr != NULL);

-        NewStrCat (OptionString[0], StringPtr);

-        Character[0] = RIGHT_ONEOF_DELIMITER;

-        NewStrCat (OptionString[0], Character);

-

-        FreePool (StringPtr);

-      }

+      FreePool (StringPtr);

     }

     break;

 

diff --git a/MdeModulePkg/Universal/SetupBrowserDxe/Setup.c b/MdeModulePkg/Universal/SetupBrowserDxe/Setup.c
index ed7f015d66..381cefeae5 100644
--- a/MdeModulePkg/Universal/SetupBrowserDxe/Setup.c
+++ b/MdeModulePkg/Universal/SetupBrowserDxe/Setup.c
@@ -3026,6 +3026,12 @@ GetQuestionDefault (
       Link = GetFirstNode (&Question->OptionListHead);

       while (!IsNull (&Question->OptionListHead, Link)) {

         Option = QUESTION_OPTION_FROM_LINK (Link);

+        Link = GetNextNode (&Question->OptionListHead, Link);

+

+        if ((Option->SuppressExpression != NULL) &&

+            EvaluateExpressionList(Option->SuppressExpression, FALSE, NULL, NULL) != ExpressFalse) {

+          continue;

+        }

 

         if (((DefaultId == EFI_HII_DEFAULT_CLASS_STANDARD) && ((Option->Flags & EFI_IFR_OPTION_DEFAULT) != 0)) ||

             ((DefaultId == EFI_HII_DEFAULT_CLASS_MANUFACTURING) && ((Option->Flags & EFI_IFR_OPTION_DEFAULT_MFG) != 0))

@@ -3034,8 +3040,6 @@ GetQuestionDefault (
 

           return EFI_SUCCESS;

         }

-

-        Link = GetNextNode (&Question->OptionListHead, Link);

       }

     }

   }

@@ -3081,10 +3085,18 @@ GetQuestionDefault (
     //

     if (ValueToOption (Question, HiiValue) == NULL) {

       Link = GetFirstNode (&Question->OptionListHead);

-      if (!IsNull (&Question->OptionListHead, Link)) {

+      while (!IsNull (&Question->OptionListHead, Link)) {

         Option = QUESTION_OPTION_FROM_LINK (Link);

+        Link = GetNextNode (&Question->OptionListHead, Link);

+

+        if ((Option->SuppressExpression != NULL) &&

+            EvaluateExpressionList(Option->SuppressExpression, FALSE, NULL, NULL) != ExpressFalse) {

+          continue;

+        }

+

         CopyMem (HiiValue, &Option->Value, sizeof (EFI_HII_VALUE));

         Status = EFI_SUCCESS;

+        break;

       }

     }

     break;

@@ -3098,6 +3110,12 @@ GetQuestionDefault (
     while (!IsNull (&Question->OptionListHead, Link)) {

       Status = EFI_SUCCESS;

       Option = QUESTION_OPTION_FROM_LINK (Link);

+      Link = GetNextNode (&Question->OptionListHead, Link);

+

+      if ((Option->SuppressExpression != NULL) &&

+          EvaluateExpressionList(Option->SuppressExpression, FALSE, NULL, NULL) != ExpressFalse) {

+        continue;

+      }

 

       SetArrayData (Question->BufferValue, Question->ValueType, Index, Option->Value.Value.u64);

 

@@ -3105,8 +3123,6 @@ GetQuestionDefault (
       if (Index >= Question->MaxContainers) {

         break;

       }

-

-      Link = GetNextNode (&Question->OptionListHead, Link);

     }

     break;

 

diff --git a/MdeModulePkg/Universal/SetupBrowserDxe/Ui.h b/MdeModulePkg/Universal/SetupBrowserDxe/Ui.h
index fca33b71e5..ac016bbed8 100644
--- a/MdeModulePkg/Universal/SetupBrowserDxe/Ui.h
+++ b/MdeModulePkg/Universal/SetupBrowserDxe/Ui.h
@@ -579,6 +579,26 @@ SetArrayData (
   );

 

 /**

+  Check whether this value already in the array, if yes, return the index.

+

+  @param  Array                  The data array.

+  @param  Type                   Type of the data in this array.

+  @param  Value                  The value to be find.

+  @param  Index                  The index in the array which has same value with Value.

+  

+  @retval   TRUE Found the value in the array.

+  @retval   FALSE Not found the value.

+

+**/

+BOOLEAN 

+FindArrayData (

+  IN VOID                     *Array,

+  IN UINT8                    Type,

+  IN UINT64                   Value,

+  OUT UINTN                   *Index OPTIONAL

+  );

+

+/**

   Process a Question's Option (whether selected or un-selected).

 

   @param  Selection              Pointer to UI_MENU_SELECTION.