public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH v2 0/2] Share default value if some default value are not specified
@ 2016-08-09  1:24 Dandan Bi
  2016-08-09  1:24 ` [PATCH v2 1/2] MdeModulePkg/HiiDB: Share default " Dandan Bi
  2016-08-09  1:24 ` [PATCH v2 2/2] MdeModulePkg/Browser: " Dandan Bi
  0 siblings, 2 replies; 5+ messages in thread
From: Dandan Bi @ 2016-08-09  1:24 UTC (permalink / raw)
  To: edk2-devel; +Cc: Liming Gao, Eric Dong

Add a new implementation policy of get default value in HiiDatabaseDxe
and SetupBrowserDxe.
The new policy is only for the situation that a question has default
value but doesn't have default value for all supported default type.
In this case, we will choose the smallest default id from the existing
defaults, and share its value to other default id which has no
default value.

Notes:
v1->V2:
-Patch 1: Clarify that when the type of DefaultValueData is DefaultValueFromOtherDefault, it can be overrode by itself.
-Patch 2: Generate DefaultStoreList as ascending order directly instead of using function GetDefaultIdArray().

Cc: Liming Gao <liming.gao@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Dandan Bi <dandan.bi@intel.com>

Dandan Bi (2):
  MdeModulePkg/HiiDB: Share default if some default value are not
    specified
  MdeModulePkg/Browser: Share default if some default value are not
    specified

 .../Universal/HiiDatabaseDxe/ConfigRouting.c       | 116 ++++++++++++++++++---
 .../Universal/HiiDatabaseDxe/HiiDatabase.h         |   2 +
 MdeModulePkg/Universal/SetupBrowserDxe/IfrParse.c  |  25 ++++-
 MdeModulePkg/Universal/SetupBrowserDxe/Setup.c     |  23 +++-
 4 files changed, 145 insertions(+), 21 deletions(-)

-- 
1.9.5.msysgit.1



^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v2 1/2] MdeModulePkg/HiiDB: Share default if some default value are not specified
  2016-08-09  1:24 [PATCH v2 0/2] Share default value if some default value are not specified Dandan Bi
@ 2016-08-09  1:24 ` Dandan Bi
  2016-08-10 10:02   ` Gao, Liming
  2016-08-09  1:24 ` [PATCH v2 2/2] MdeModulePkg/Browser: " Dandan Bi
  1 sibling, 1 reply; 5+ messages in thread
From: Dandan Bi @ 2016-08-09  1:24 UTC (permalink / raw)
  To: edk2-devel; +Cc: Liming Gao, Eric Dong

Add a new implementation policy of getting default value in HiiDatabase.
The new policy is only for the situation that a question has default
value but doesn't have default value for all supported default type.
In this case, we will choose the smallest default id from the existing
defaults, and share its value to other default id which has no
default value.

Notes:
v1->V2:
Clarify that when the type of DefaultValueData is
DefaultValueFromOtherDefault,it can be overrode by itself.

Cc: Liming Gao <liming.gao@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Dandan Bi <dandan.bi@intel.com>
---
 .../Universal/HiiDatabaseDxe/ConfigRouting.c       | 116 ++++++++++++++++++---
 .../Universal/HiiDatabaseDxe/HiiDatabase.h         |   2 +
 2 files changed, 102 insertions(+), 16 deletions(-)

diff --git a/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c b/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c
index 0578352..546e60c 100644
--- a/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c
+++ b/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c
@@ -1149,12 +1149,13 @@ InsertDefaultValue (
   for (Link = DefaultLink->ForwardLink; Link != DefaultLink; Link = Link->ForwardLink) {
     DefaultValueArray = BASE_CR (Link, IFR_DEFAULT_DATA, Entry);
     if (DefaultValueArray->DefaultId == DefaultValueData->DefaultId) {
       //
       // DEFAULT_VALUE_FROM_OPCODE has high priority, DEFAULT_VALUE_FROM_DEFAULT has low priority.
+      // When the two default types are DEFAULT_VALUE_FROM_OTHER_DEFAULT, the default value can be overrode.
       //
-      if (DefaultValueData->Type > DefaultValueArray->Type) {
+      if ((DefaultValueData->Type > DefaultValueArray->Type) || (DefaultValueData->Type == DefaultValueArray->Type && DefaultValueData->Type == DefaultValueFromOtherDefault)) {
         //
         // Update the default value array in BlockData.
         //
         CopyMem (&DefaultValueArray->Value, &DefaultValueData->Value, sizeof (EFI_IFR_TYPE_VALUE));
         DefaultValueArray->Type  = DefaultValueData->Type;
@@ -2099,19 +2100,22 @@ ParseIfrData (
   LIST_ENTRY               *LinkData;
   LIST_ENTRY               *LinkDefault;
   EFI_IFR_VARSTORE_NAME_VALUE *IfrNameValueVarStore;
   EFI_HII_PACKAGE_HEADER   *PackageHeader;
   EFI_VARSTORE_ID          VarStoreId;
+  UINT16                   SmallestDefaultId;
+  UINT16                   SmallestIdFromFlag;
 
   Status           = EFI_SUCCESS;
   BlockData        = NULL;
   DefaultDataPtr   = NULL;
   FirstOneOfOption = FALSE;
   VarStoreId       = 0;
   FirstOrderedList = FALSE;
   VarStoreName     = NULL;
   ZeroMem (&DefaultData, sizeof (IFR_DEFAULT_DATA));
+  SmallestDefaultId = 0xFFFF;
 
   //
   // Go through the form package to parse OpCode one by one.
   //
   PackageOffset = sizeof (EFI_HII_PACKAGE_HEADER);
@@ -2473,10 +2477,12 @@ ParseIfrData (
       //
       //when go to there,BlockData can't be NULLL.
       //
       ASSERT (BlockData != NULL);
 
+      SmallestIdFromFlag = FALSE;
+
       //
       // Add default value for standard ID by CheckBox Flag
       //
       VarDefaultId = EFI_HII_DEFAULT_CLASS_STANDARD;
       //
@@ -2487,21 +2493,20 @@ ParseIfrData (
         //
         // When flag is set, defautl value is TRUE.
         //
         DefaultData.Type    = DefaultValueFromFlag;
         DefaultData.Value.b = TRUE;
-      } else {
-        //
-        // When flag is not set, defautl value is FASLE.
-        //
-        DefaultData.Type    = DefaultValueFromDefault;
-        DefaultData.Value.b = FALSE;
+        InsertDefaultValue (BlockData, &DefaultData);
+
+        if (SmallestDefaultId > EFI_HII_DEFAULT_CLASS_STANDARD) {
+          //
+          // Record the SmallestDefaultId and update the SmallestIdFromFlag.
+          //
+          SmallestDefaultId = EFI_HII_DEFAULT_CLASS_STANDARD;
+          SmallestIdFromFlag = TRUE;
+        }
       }
-      //
-      // Add DefaultValue into current BlockData
-      //
-      InsertDefaultValue (BlockData, &DefaultData);
 
       //
       // Add default value for Manufacture ID by CheckBox Flag
       //
       VarDefaultId = EFI_HII_DEFAULT_CLASS_MANUFACTURING;
@@ -2513,21 +2518,49 @@ ParseIfrData (
         //
         // When flag is set, defautl value is TRUE.
         //
         DefaultData.Type    = DefaultValueFromFlag;
         DefaultData.Value.b = TRUE;
+        InsertDefaultValue (BlockData, &DefaultData);
+
+        if (SmallestDefaultId > EFI_HII_DEFAULT_CLASS_MANUFACTURING) {
+          //
+          // Record the SmallestDefaultId and update the SmallestIdFromFlag.
+          //
+          SmallestDefaultId = EFI_HII_DEFAULT_CLASS_MANUFACTURING;
+          SmallestIdFromFlag = TRUE;
+        }
+      }
+      if (SmallestIdFromFlag) {
+        //
+        // When smallest default Id is given by the  flag of CheckBox, set defaut value with TRUE for other default Id in the DefaultId list.
+        //
+        DefaultData.Type    = DefaultValueFromOtherDefault;
+        DefaultData.Value.b = TRUE;
+        //
+        // Set default value for all the default id in the DefaultId list.
+        //
+        for (LinkData = DefaultIdArray->Entry.ForwardLink; LinkData != &DefaultIdArray->Entry; LinkData = LinkData->ForwardLink) {
+          DefaultDataPtr = BASE_CR (LinkData, IFR_DEFAULT_DATA, Entry);
+          DefaultData.DefaultId   = DefaultDataPtr->DefaultId;
+          InsertDefaultValue (BlockData, &DefaultData);
+        }
       } else {
         //
         // When flag is not set, defautl value is FASLE.
         //
         DefaultData.Type    = DefaultValueFromDefault;
         DefaultData.Value.b = FALSE;
+        //
+        // Set default value for all the default id in the DefaultId list.
+        //
+        for (LinkData = DefaultIdArray->Entry.ForwardLink; LinkData != &DefaultIdArray->Entry; LinkData = LinkData->ForwardLink) {
+          DefaultDataPtr = BASE_CR (LinkData, IFR_DEFAULT_DATA, Entry);
+          DefaultData.DefaultId   = DefaultDataPtr->DefaultId;
+          InsertDefaultValue (BlockData, &DefaultData);
+        }
       }
-      //
-      // Add DefaultValue into current BlockData
-      //
-      InsertDefaultValue (BlockData, &DefaultData);
       break;
 
     case EFI_IFR_DATE_OP:
       //
       // offset by question header
@@ -2777,30 +2810,62 @@ ParseIfrData (
         break;
       }
 
       //
       // 1. Set default value for OneOf option when flag field has default attribute.
+      //    And set the default value with the smallest default id for other default id in the DefaultId list.
       //
       if (((IfrOneOfOption->Flags & EFI_IFR_OPTION_DEFAULT) == EFI_IFR_OPTION_DEFAULT) ||
           ((IfrOneOfOption->Flags & EFI_IFR_OPTION_DEFAULT_MFG) == EFI_IFR_OPTION_DEFAULT_MFG)) {
         //
         // This flag is used to specify whether this option is the first. Set it to FALSE for the following options. 
         // The first oneof option value will be used as default value when no default value is specified. 
         //
         FirstOneOfOption = FALSE;
+
+        SmallestIdFromFlag = FALSE;
         
         // Prepare new DefaultValue
         //
         DefaultData.Type     = DefaultValueFromFlag;
         CopyMem (&DefaultData.Value, &IfrOneOfOption->Value, IfrOneOfOption->Header.Length - OFFSET_OF (EFI_IFR_ONE_OF_OPTION, Value));
         if ((IfrOneOfOption->Flags & EFI_IFR_OPTION_DEFAULT) == EFI_IFR_OPTION_DEFAULT) {
           DefaultData.DefaultId = EFI_HII_DEFAULT_CLASS_STANDARD;
           InsertDefaultValue (BlockData, &DefaultData);
-        } 
+          if (SmallestDefaultId > EFI_HII_DEFAULT_CLASS_STANDARD) {
+            //
+            // Record the SmallestDefaultId and update the SmallestIdFromFlag.
+            //
+            SmallestDefaultId = EFI_HII_DEFAULT_CLASS_STANDARD;
+            SmallestIdFromFlag = TRUE;
+          }
+        }
         if ((IfrOneOfOption->Flags & EFI_IFR_OPTION_DEFAULT_MFG) == EFI_IFR_OPTION_DEFAULT_MFG) {
           DefaultData.DefaultId = EFI_HII_DEFAULT_CLASS_MANUFACTURING;
           InsertDefaultValue (BlockData, &DefaultData);
+          if (SmallestDefaultId > EFI_HII_DEFAULT_CLASS_MANUFACTURING) {
+            //
+            // Record the SmallestDefaultId and update the SmallestIdFromFlag.
+            //
+            SmallestDefaultId = EFI_HII_DEFAULT_CLASS_MANUFACTURING;
+            SmallestIdFromFlag = TRUE;
+          }
+        }
+
+        if (SmallestIdFromFlag) {
+          //
+          // When smallest default Id is given by the flag of oneofOption, set this option value for other default Id in the DefaultId list.
+          //
+          DefaultData.Type = DefaultValueFromOtherDefault;
+          //
+          // Set default value for other default id in the DefaultId list.
+          //
+          for (LinkData = DefaultIdArray->Entry.ForwardLink; LinkData != &DefaultIdArray->Entry; LinkData = LinkData->ForwardLink) {
+            DefaultDataPtr = BASE_CR (LinkData, IFR_DEFAULT_DATA, Entry);
+            DefaultData.DefaultId   = DefaultDataPtr->DefaultId;
+            InsertDefaultValue (BlockData, &DefaultData);
+          }
         }
       }
 
       //
       // 2. Set as the default value when this is the first option.
@@ -2854,10 +2919,25 @@ ParseIfrData (
       // Add DefaultValue into current BlockData
       //
       InsertDefaultValue (BlockData, &DefaultData);
 
       //
+      // Set default value for other default id in the DefaultId list.
+      //
+     if (SmallestDefaultId >= VarDefaultId) {
+        SmallestDefaultId = VarDefaultId;
+        for (LinkData = DefaultIdArray->Entry.ForwardLink; LinkData != &DefaultIdArray->Entry; LinkData = LinkData->ForwardLink) {
+          DefaultDataPtr = BASE_CR (LinkData, IFR_DEFAULT_DATA, Entry);
+          if (DefaultDataPtr->DefaultId != DefaultData.DefaultId){
+            DefaultData.Type        = DefaultValueFromOtherDefault;
+            DefaultData.DefaultId   = DefaultDataPtr->DefaultId;
+            InsertDefaultValue (BlockData, &DefaultData);
+          }
+        }
+      }
+
+      //
       // After insert the default value, reset the cleaned value for next 
       // time used. If not set here, need to set the value before everytime 
       // use it.
       //
       DefaultData.Cleaned     = FALSE;
@@ -2871,10 +2951,14 @@ ParseIfrData (
         if (BlockData->Scope > 0) {
           BlockData->Scope--;
         }
         if (BlockData->Scope == 0) {
           BlockData = NULL;
+          //
+          // when finishing parsing a question, clean the SmallestDefaultId of the question.
+          //
+          SmallestDefaultId = 0xFFFF;
         }
       }
 
       break;
 
diff --git a/MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabase.h b/MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabase.h
index d90bc02..1b0f7f6 100644
--- a/MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabase.h
+++ b/MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabase.h
@@ -85,10 +85,12 @@ typedef struct {
 //
 // Get default value from IFR data.
 //
 typedef enum {
   DefaultValueFromDefault = 0,     // Get from the minimum or first one when not set default value.
+  DefaultValueFromOtherDefault,    // Get default vale from other default when no default(When other
+                                   // defaults are more than one, use the default with smallest default id).
   DefaultValueFromFlag,            // Get default value from the defalut flag.
   DefaultValueFromOpcode           // Get default value from default opcode, highest priority.
 } DEFAULT_VALUE_TYPE;
 
 typedef struct {
-- 
1.9.5.msysgit.1



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH v2 2/2] MdeModulePkg/Browser: Share default if some default value are not specified
  2016-08-09  1:24 [PATCH v2 0/2] Share default value if some default value are not specified Dandan Bi
  2016-08-09  1:24 ` [PATCH v2 1/2] MdeModulePkg/HiiDB: Share default " Dandan Bi
@ 2016-08-09  1:24 ` Dandan Bi
  2016-08-10 12:26   ` Gao, Liming
  1 sibling, 1 reply; 5+ messages in thread
From: Dandan Bi @ 2016-08-09  1:24 UTC (permalink / raw)
  To: edk2-devel; +Cc: Liming Gao, Eric Dong

Add a new implementation policy of getting default value in SetupBrowser.
The new policy is only for the situation that a question has default
value but doesn't have default value for all supported default type.
In this case, we will choose the smallest default id from the existing
defaults, and share its value to other default id which has no
default value.

Notes:
v1->V2:
Generate DefaultStoreList as ascending order directly instead
of using function GetDefaultIdArray().

Cc: Liming Gao <liming.gao@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Dandan Bi <dandan.bi@intel.com>
---
 MdeModulePkg/Universal/SetupBrowserDxe/IfrParse.c | 25 +++++++++++++++++++----
 MdeModulePkg/Universal/SetupBrowserDxe/Setup.c    | 23 ++++++++++++++++++++-
 2 files changed, 43 insertions(+), 5 deletions(-)

diff --git a/MdeModulePkg/Universal/SetupBrowserDxe/IfrParse.c b/MdeModulePkg/Universal/SetupBrowserDxe/IfrParse.c
index 11a8fdc..61ba0b5 100644
--- a/MdeModulePkg/Universal/SetupBrowserDxe/IfrParse.c
+++ b/MdeModulePkg/Universal/SetupBrowserDxe/IfrParse.c
@@ -1,9 +1,9 @@
 /** @file
 Parser for IFR binary encoding.
 
-Copyright (c) 2007 - 2015, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2007 - 2016, Intel Corporation. All rights reserved.<BR>
 This program and the accompanying materials
 are licensed and made available under the terms and conditions of the BSD License
 which accompanies this distribution.  The full text of the license may be found at
 http://opensource.org/licenses/bsd-license.php
 
@@ -1309,10 +1309,13 @@ ParseOpCodes (
   EFI_VARSTORE_ID         TempVarstoreId;
   BOOLEAN                 InScopeDisable;
   INTN                    ConditionalExprCount;
   BOOLEAN                 InUnknownScope;
   UINT8                   UnknownDepth;
+  FORMSET_DEFAULTSTORE    *PreDefaultStore;
+  LIST_ENTRY              *DefaultLink;
+  BOOLEAN                 HaveInserted;
 
   SuppressForQuestion      = FALSE;
   SuppressForOption        = FALSE;
   InScopeDisable           = FALSE;
   DepthOfDisable           = 0;
@@ -1873,21 +1876,35 @@ ParseOpCodes (
 
     //
     // DefaultStore
     //
     case EFI_IFR_DEFAULTSTORE_OP:
+      HaveInserted = FALSE;
       DefaultStore = AllocateZeroPool (sizeof (FORMSET_DEFAULTSTORE));
       ASSERT (DefaultStore != NULL);
       DefaultStore->Signature = FORMSET_DEFAULTSTORE_SIGNATURE;
 
       CopyMem (&DefaultStore->DefaultId,   &((EFI_IFR_DEFAULTSTORE *) OpCodeData)->DefaultId,   sizeof (UINT16));
       CopyMem (&DefaultStore->DefaultName, &((EFI_IFR_DEFAULTSTORE *) OpCodeData)->DefaultName, sizeof (EFI_STRING_ID));
-
       //
-      // Insert to DefaultStore list of this Formset
+      // Insert it to the DefaultStore list of this Formset with ascending order.
       //
-      InsertTailList (&FormSet->DefaultStoreListHead, &DefaultStore->Link);
+      if (!IsListEmpty (&FormSet->DefaultStoreListHead)) {
+        DefaultLink = GetFirstNode (&FormSet->DefaultStoreListHead);
+        while (!IsNull (&FormSet->DefaultStoreListHead, DefaultLink)) {
+          PreDefaultStore = FORMSET_DEFAULTSTORE_FROM_LINK(DefaultLink);
+          DefaultLink = GetNextNode (&FormSet->DefaultStoreListHead, DefaultLink);
+          if (DefaultStore->DefaultId < PreDefaultStore->DefaultId) {
+            InsertTailList (&PreDefaultStore->Link, &DefaultStore->Link);
+            HaveInserted = TRUE;
+            break;
+          }
+        }
+      }
+      if (!HaveInserted) {
+        InsertTailList (&FormSet->DefaultStoreListHead, &DefaultStore->Link);
+      }
       break;
 
     //
     // Statements
     //
diff --git a/MdeModulePkg/Universal/SetupBrowserDxe/Setup.c b/MdeModulePkg/Universal/SetupBrowserDxe/Setup.c
index 6b38547..66c4313 100644
--- a/MdeModulePkg/Universal/SetupBrowserDxe/Setup.c
+++ b/MdeModulePkg/Universal/SetupBrowserDxe/Setup.c
@@ -4048,13 +4048,18 @@ GetQuestionDefault (
   EFI_HII_CONFIG_ACCESS_PROTOCOL  *ConfigAccess;
   EFI_BROWSER_ACTION_REQUEST      ActionRequest;
   INTN                            Action;
   CHAR16                          *NewString;
   EFI_IFR_TYPE_VALUE              *TypeValue;
+  UINT16                          OriginalDefaultId;
+  FORMSET_DEFAULTSTORE            *DefaultStore;
+  LIST_ENTRY                      *DefaultLink;
 
   Status   = EFI_NOT_FOUND;
   StrValue = NULL;
+  OriginalDefaultId  = DefaultId;
+  DefaultLink        = GetFirstNode (&FormSet->DefaultStoreListHead);
 
   //
   // Statement don't have storage, skip them
   //
   if (Question->QuestionId == 0) {
@@ -4067,10 +4072,11 @@ GetQuestionDefault (
   //  2, use ExtractConfig function
   //  3, use nested EFI_IFR_DEFAULT 
   //  4, set flags of EFI_ONE_OF_OPTION (provide Standard and Manufacturing default)
   //  5, set flags of EFI_IFR_CHECKBOX (provide Standard and Manufacturing default) (lowest priority)
   //
+ReGetDefault:
   HiiValue = &Question->HiiValue;
   TypeValue = &HiiValue->Value;
   if (HiiValue->Type == EFI_IFR_TYPE_BUFFER) {
     //
     // For orderedlist, need to pass the BufferValue to Callback function.
@@ -4233,11 +4239,26 @@ GetQuestionDefault (
       return EFI_SUCCESS;
     }
   }
 
   //
-  // For Questions without default
+  // For question without default value for current default Id, we try to re-get the default value form other default id in the DefaultStoreList.
+  // If get, will exit the function, if not, will choose next default id in the DefaultStoreList.
+  // The default id in DefaultStoreList are in ascending order to make sure choose the smallest default id every time.
+  //
+  while (!IsNull(&FormSet->DefaultStoreListHead, DefaultLink)) {
+    DefaultStore = FORMSET_DEFAULTSTORE_FROM_LINK(DefaultLink);
+    DefaultLink = GetNextNode (&FormSet->DefaultStoreListHead,DefaultLink);
+    DefaultId = DefaultStore->DefaultId;
+    if (DefaultId == OriginalDefaultId) {
+      continue;
+    }
+    goto ReGetDefault;
+  }
+
+  //
+  // For Questions without default value for all the default id in the DefaultStoreList.
   //
   Status = EFI_NOT_FOUND;
   switch (Question->Operand) {
   case EFI_IFR_NUMERIC_OP:
     //
-- 
1.9.5.msysgit.1



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v2 1/2] MdeModulePkg/HiiDB: Share default if some default value are not specified
  2016-08-09  1:24 ` [PATCH v2 1/2] MdeModulePkg/HiiDB: Share default " Dandan Bi
@ 2016-08-10 10:02   ` Gao, Liming
  0 siblings, 0 replies; 5+ messages in thread
From: Gao, Liming @ 2016-08-10 10:02 UTC (permalink / raw)
  To: Bi, Dandan, edk2-devel@lists.01.org; +Cc: Dong, Eric

Reviewed-by: Liming Gao <liming.gao@intel.com>

> -----Original Message-----
> From: Bi, Dandan
> Sent: Tuesday, August 09, 2016 9:25 AM
> To: edk2-devel@lists.01.org
> Cc: Gao, Liming <liming.gao@intel.com>; Dong, Eric <eric.dong@intel.com>
> Subject: [PATCH v2 1/2] MdeModulePkg/HiiDB: Share default if some default
> value are not specified
> 
> Add a new implementation policy of getting default value in HiiDatabase.
> The new policy is only for the situation that a question has default
> value but doesn't have default value for all supported default type.
> In this case, we will choose the smallest default id from the existing
> defaults, and share its value to other default id which has no
> default value.
> 
> Notes:
> v1->V2:
> Clarify that when the type of DefaultValueData is
> DefaultValueFromOtherDefault,it can be overrode by itself.
> 
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Dandan Bi <dandan.bi@intel.com>
> ---
>  .../Universal/HiiDatabaseDxe/ConfigRouting.c       | 116
> ++++++++++++++++++---
>  .../Universal/HiiDatabaseDxe/HiiDatabase.h         |   2 +
>  2 files changed, 102 insertions(+), 16 deletions(-)
> 
> diff --git a/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c
> b/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c
> index 0578352..546e60c 100644
> --- a/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c
> +++ b/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c
> @@ -1149,12 +1149,13 @@ InsertDefaultValue (
>    for (Link = DefaultLink->ForwardLink; Link != DefaultLink; Link = Link-
> >ForwardLink) {
>      DefaultValueArray = BASE_CR (Link, IFR_DEFAULT_DATA, Entry);
>      if (DefaultValueArray->DefaultId == DefaultValueData->DefaultId) {
>        //
>        // DEFAULT_VALUE_FROM_OPCODE has high priority,
> DEFAULT_VALUE_FROM_DEFAULT has low priority.
> +      // When the two default types are
> DEFAULT_VALUE_FROM_OTHER_DEFAULT, the default value can be
> overrode.
>        //
> -      if (DefaultValueData->Type > DefaultValueArray->Type) {
> +      if ((DefaultValueData->Type > DefaultValueArray->Type) ||
> (DefaultValueData->Type == DefaultValueArray->Type &&
> DefaultValueData->Type == DefaultValueFromOtherDefault)) {
>          //
>          // Update the default value array in BlockData.
>          //
>          CopyMem (&DefaultValueArray->Value, &DefaultValueData->Value,
> sizeof (EFI_IFR_TYPE_VALUE));
>          DefaultValueArray->Type  = DefaultValueData->Type;
> @@ -2099,19 +2100,22 @@ ParseIfrData (
>    LIST_ENTRY               *LinkData;
>    LIST_ENTRY               *LinkDefault;
>    EFI_IFR_VARSTORE_NAME_VALUE *IfrNameValueVarStore;
>    EFI_HII_PACKAGE_HEADER   *PackageHeader;
>    EFI_VARSTORE_ID          VarStoreId;
> +  UINT16                   SmallestDefaultId;
> +  UINT16                   SmallestIdFromFlag;
> 
>    Status           = EFI_SUCCESS;
>    BlockData        = NULL;
>    DefaultDataPtr   = NULL;
>    FirstOneOfOption = FALSE;
>    VarStoreId       = 0;
>    FirstOrderedList = FALSE;
>    VarStoreName     = NULL;
>    ZeroMem (&DefaultData, sizeof (IFR_DEFAULT_DATA));
> +  SmallestDefaultId = 0xFFFF;
> 
>    //
>    // Go through the form package to parse OpCode one by one.
>    //
>    PackageOffset = sizeof (EFI_HII_PACKAGE_HEADER);
> @@ -2473,10 +2477,12 @@ ParseIfrData (
>        //
>        //when go to there,BlockData can't be NULLL.
>        //
>        ASSERT (BlockData != NULL);
> 
> +      SmallestIdFromFlag = FALSE;
> +
>        //
>        // Add default value for standard ID by CheckBox Flag
>        //
>        VarDefaultId = EFI_HII_DEFAULT_CLASS_STANDARD;
>        //
> @@ -2487,21 +2493,20 @@ ParseIfrData (
>          //
>          // When flag is set, defautl value is TRUE.
>          //
>          DefaultData.Type    = DefaultValueFromFlag;
>          DefaultData.Value.b = TRUE;
> -      } else {
> -        //
> -        // When flag is not set, defautl value is FASLE.
> -        //
> -        DefaultData.Type    = DefaultValueFromDefault;
> -        DefaultData.Value.b = FALSE;
> +        InsertDefaultValue (BlockData, &DefaultData);
> +
> +        if (SmallestDefaultId > EFI_HII_DEFAULT_CLASS_STANDARD) {
> +          //
> +          // Record the SmallestDefaultId and update the SmallestIdFromFlag.
> +          //
> +          SmallestDefaultId = EFI_HII_DEFAULT_CLASS_STANDARD;
> +          SmallestIdFromFlag = TRUE;
> +        }
>        }
> -      //
> -      // Add DefaultValue into current BlockData
> -      //
> -      InsertDefaultValue (BlockData, &DefaultData);
> 
>        //
>        // Add default value for Manufacture ID by CheckBox Flag
>        //
>        VarDefaultId = EFI_HII_DEFAULT_CLASS_MANUFACTURING;
> @@ -2513,21 +2518,49 @@ ParseIfrData (
>          //
>          // When flag is set, defautl value is TRUE.
>          //
>          DefaultData.Type    = DefaultValueFromFlag;
>          DefaultData.Value.b = TRUE;
> +        InsertDefaultValue (BlockData, &DefaultData);
> +
> +        if (SmallestDefaultId > EFI_HII_DEFAULT_CLASS_MANUFACTURING) {
> +          //
> +          // Record the SmallestDefaultId and update the SmallestIdFromFlag.
> +          //
> +          SmallestDefaultId = EFI_HII_DEFAULT_CLASS_MANUFACTURING;
> +          SmallestIdFromFlag = TRUE;
> +        }
> +      }
> +      if (SmallestIdFromFlag) {
> +        //
> +        // When smallest default Id is given by the  flag of CheckBox, set defaut
> value with TRUE for other default Id in the DefaultId list.
> +        //
> +        DefaultData.Type    = DefaultValueFromOtherDefault;
> +        DefaultData.Value.b = TRUE;
> +        //
> +        // Set default value for all the default id in the DefaultId list.
> +        //
> +        for (LinkData = DefaultIdArray->Entry.ForwardLink; LinkData !=
> &DefaultIdArray->Entry; LinkData = LinkData->ForwardLink) {
> +          DefaultDataPtr = BASE_CR (LinkData, IFR_DEFAULT_DATA, Entry);
> +          DefaultData.DefaultId   = DefaultDataPtr->DefaultId;
> +          InsertDefaultValue (BlockData, &DefaultData);
> +        }
>        } else {
>          //
>          // When flag is not set, defautl value is FASLE.
>          //
>          DefaultData.Type    = DefaultValueFromDefault;
>          DefaultData.Value.b = FALSE;
> +        //
> +        // Set default value for all the default id in the DefaultId list.
> +        //
> +        for (LinkData = DefaultIdArray->Entry.ForwardLink; LinkData !=
> &DefaultIdArray->Entry; LinkData = LinkData->ForwardLink) {
> +          DefaultDataPtr = BASE_CR (LinkData, IFR_DEFAULT_DATA, Entry);
> +          DefaultData.DefaultId   = DefaultDataPtr->DefaultId;
> +          InsertDefaultValue (BlockData, &DefaultData);
> +        }
>        }
> -      //
> -      // Add DefaultValue into current BlockData
> -      //
> -      InsertDefaultValue (BlockData, &DefaultData);
>        break;
> 
>      case EFI_IFR_DATE_OP:
>        //
>        // offset by question header
> @@ -2777,30 +2810,62 @@ ParseIfrData (
>          break;
>        }
> 
>        //
>        // 1. Set default value for OneOf option when flag field has default
> attribute.
> +      //    And set the default value with the smallest default id for other
> default id in the DefaultId list.
>        //
>        if (((IfrOneOfOption->Flags & EFI_IFR_OPTION_DEFAULT) ==
> EFI_IFR_OPTION_DEFAULT) ||
>            ((IfrOneOfOption->Flags & EFI_IFR_OPTION_DEFAULT_MFG) ==
> EFI_IFR_OPTION_DEFAULT_MFG)) {
>          //
>          // This flag is used to specify whether this option is the first. Set it to
> FALSE for the following options.
>          // The first oneof option value will be used as default value when no
> default value is specified.
>          //
>          FirstOneOfOption = FALSE;
> +
> +        SmallestIdFromFlag = FALSE;
> 
>          // Prepare new DefaultValue
>          //
>          DefaultData.Type     = DefaultValueFromFlag;
>          CopyMem (&DefaultData.Value, &IfrOneOfOption->Value,
> IfrOneOfOption->Header.Length - OFFSET_OF (EFI_IFR_ONE_OF_OPTION,
> Value));
>          if ((IfrOneOfOption->Flags & EFI_IFR_OPTION_DEFAULT) ==
> EFI_IFR_OPTION_DEFAULT) {
>            DefaultData.DefaultId = EFI_HII_DEFAULT_CLASS_STANDARD;
>            InsertDefaultValue (BlockData, &DefaultData);
> -        }
> +          if (SmallestDefaultId > EFI_HII_DEFAULT_CLASS_STANDARD) {
> +            //
> +            // Record the SmallestDefaultId and update the SmallestIdFromFlag.
> +            //
> +            SmallestDefaultId = EFI_HII_DEFAULT_CLASS_STANDARD;
> +            SmallestIdFromFlag = TRUE;
> +          }
> +        }
>          if ((IfrOneOfOption->Flags & EFI_IFR_OPTION_DEFAULT_MFG) ==
> EFI_IFR_OPTION_DEFAULT_MFG) {
>            DefaultData.DefaultId = EFI_HII_DEFAULT_CLASS_MANUFACTURING;
>            InsertDefaultValue (BlockData, &DefaultData);
> +          if (SmallestDefaultId > EFI_HII_DEFAULT_CLASS_MANUFACTURING) {
> +            //
> +            // Record the SmallestDefaultId and update the SmallestIdFromFlag.
> +            //
> +            SmallestDefaultId = EFI_HII_DEFAULT_CLASS_MANUFACTURING;
> +            SmallestIdFromFlag = TRUE;
> +          }
> +        }
> +
> +        if (SmallestIdFromFlag) {
> +          //
> +          // When smallest default Id is given by the flag of oneofOption, set this
> option value for other default Id in the DefaultId list.
> +          //
> +          DefaultData.Type = DefaultValueFromOtherDefault;
> +          //
> +          // Set default value for other default id in the DefaultId list.
> +          //
> +          for (LinkData = DefaultIdArray->Entry.ForwardLink; LinkData !=
> &DefaultIdArray->Entry; LinkData = LinkData->ForwardLink) {
> +            DefaultDataPtr = BASE_CR (LinkData, IFR_DEFAULT_DATA, Entry);
> +            DefaultData.DefaultId   = DefaultDataPtr->DefaultId;
> +            InsertDefaultValue (BlockData, &DefaultData);
> +          }
>          }
>        }
> 
>        //
>        // 2. Set as the default value when this is the first option.
> @@ -2854,10 +2919,25 @@ ParseIfrData (
>        // Add DefaultValue into current BlockData
>        //
>        InsertDefaultValue (BlockData, &DefaultData);
> 
>        //
> +      // Set default value for other default id in the DefaultId list.
> +      //
> +     if (SmallestDefaultId >= VarDefaultId) {
> +        SmallestDefaultId = VarDefaultId;
> +        for (LinkData = DefaultIdArray->Entry.ForwardLink; LinkData !=
> &DefaultIdArray->Entry; LinkData = LinkData->ForwardLink) {
> +          DefaultDataPtr = BASE_CR (LinkData, IFR_DEFAULT_DATA, Entry);
> +          if (DefaultDataPtr->DefaultId != DefaultData.DefaultId){
> +            DefaultData.Type        = DefaultValueFromOtherDefault;
> +            DefaultData.DefaultId   = DefaultDataPtr->DefaultId;
> +            InsertDefaultValue (BlockData, &DefaultData);
> +          }
> +        }
> +      }
> +
> +      //
>        // After insert the default value, reset the cleaned value for next
>        // time used. If not set here, need to set the value before everytime
>        // use it.
>        //
>        DefaultData.Cleaned     = FALSE;
> @@ -2871,10 +2951,14 @@ ParseIfrData (
>          if (BlockData->Scope > 0) {
>            BlockData->Scope--;
>          }
>          if (BlockData->Scope == 0) {
>            BlockData = NULL;
> +          //
> +          // when finishing parsing a question, clean the SmallestDefaultId of the
> question.
> +          //
> +          SmallestDefaultId = 0xFFFF;
>          }
>        }
> 
>        break;
> 
> diff --git a/MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabase.h
> b/MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabase.h
> index d90bc02..1b0f7f6 100644
> --- a/MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabase.h
> +++ b/MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabase.h
> @@ -85,10 +85,12 @@ typedef struct {
>  //
>  // Get default value from IFR data.
>  //
>  typedef enum {
>    DefaultValueFromDefault = 0,     // Get from the minimum or first one when
> not set default value.
> +  DefaultValueFromOtherDefault,    // Get default vale from other default
> when no default(When other
> +                                   // defaults are more than one, use the default with
> smallest default id).
>    DefaultValueFromFlag,            // Get default value from the defalut flag.
>    DefaultValueFromOpcode           // Get default value from default opcode,
> highest priority.
>  } DEFAULT_VALUE_TYPE;
> 
>  typedef struct {
> --
> 1.9.5.msysgit.1



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2 2/2] MdeModulePkg/Browser: Share default if some default value are not specified
  2016-08-09  1:24 ` [PATCH v2 2/2] MdeModulePkg/Browser: " Dandan Bi
@ 2016-08-10 12:26   ` Gao, Liming
  0 siblings, 0 replies; 5+ messages in thread
From: Gao, Liming @ 2016-08-10 12:26 UTC (permalink / raw)
  To: Bi, Dandan, edk2-devel@lists.01.org; +Cc: Dong, Eric

Reviewed-by: Liming Gao <liming.gao@intel.com>

> -----Original Message-----
> From: Bi, Dandan
> Sent: Tuesday, August 09, 2016 9:25 AM
> To: edk2-devel@lists.01.org
> Cc: Gao, Liming <liming.gao@intel.com>; Dong, Eric <eric.dong@intel.com>
> Subject: [PATCH v2 2/2] MdeModulePkg/Browser: Share default if some
> default value are not specified
> 
> Add a new implementation policy of getting default value in SetupBrowser.
> The new policy is only for the situation that a question has default
> value but doesn't have default value for all supported default type.
> In this case, we will choose the smallest default id from the existing
> defaults, and share its value to other default id which has no
> default value.
> 
> Notes:
> v1->V2:
> Generate DefaultStoreList as ascending order directly instead
> of using function GetDefaultIdArray().
> 
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Dandan Bi <dandan.bi@intel.com>
> ---
>  MdeModulePkg/Universal/SetupBrowserDxe/IfrParse.c | 25
> +++++++++++++++++++----
>  MdeModulePkg/Universal/SetupBrowserDxe/Setup.c    | 23
> ++++++++++++++++++++-
>  2 files changed, 43 insertions(+), 5 deletions(-)
> 
> diff --git a/MdeModulePkg/Universal/SetupBrowserDxe/IfrParse.c
> b/MdeModulePkg/Universal/SetupBrowserDxe/IfrParse.c
> index 11a8fdc..61ba0b5 100644
> --- a/MdeModulePkg/Universal/SetupBrowserDxe/IfrParse.c
> +++ b/MdeModulePkg/Universal/SetupBrowserDxe/IfrParse.c
> @@ -1,9 +1,9 @@
>  /** @file
>  Parser for IFR binary encoding.
> 
> -Copyright (c) 2007 - 2015, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2007 - 2016, Intel Corporation. All rights reserved.<BR>
>  This program and the accompanying materials
>  are licensed and made available under the terms and conditions of the BSD
> License
>  which accompanies this distribution.  The full text of the license may be
> found at
>  http://opensource.org/licenses/bsd-license.php
> 
> @@ -1309,10 +1309,13 @@ ParseOpCodes (
>    EFI_VARSTORE_ID         TempVarstoreId;
>    BOOLEAN                 InScopeDisable;
>    INTN                    ConditionalExprCount;
>    BOOLEAN                 InUnknownScope;
>    UINT8                   UnknownDepth;
> +  FORMSET_DEFAULTSTORE    *PreDefaultStore;
> +  LIST_ENTRY              *DefaultLink;
> +  BOOLEAN                 HaveInserted;
> 
>    SuppressForQuestion      = FALSE;
>    SuppressForOption        = FALSE;
>    InScopeDisable           = FALSE;
>    DepthOfDisable           = 0;
> @@ -1873,21 +1876,35 @@ ParseOpCodes (
> 
>      //
>      // DefaultStore
>      //
>      case EFI_IFR_DEFAULTSTORE_OP:
> +      HaveInserted = FALSE;
>        DefaultStore = AllocateZeroPool (sizeof (FORMSET_DEFAULTSTORE));
>        ASSERT (DefaultStore != NULL);
>        DefaultStore->Signature = FORMSET_DEFAULTSTORE_SIGNATURE;
> 
>        CopyMem (&DefaultStore->DefaultId,   &((EFI_IFR_DEFAULTSTORE *)
> OpCodeData)->DefaultId,   sizeof (UINT16));
>        CopyMem (&DefaultStore->DefaultName, &((EFI_IFR_DEFAULTSTORE *)
> OpCodeData)->DefaultName, sizeof (EFI_STRING_ID));
> -
>        //
> -      // Insert to DefaultStore list of this Formset
> +      // Insert it to the DefaultStore list of this Formset with ascending order.
>        //
> -      InsertTailList (&FormSet->DefaultStoreListHead, &DefaultStore->Link);
> +      if (!IsListEmpty (&FormSet->DefaultStoreListHead)) {
> +        DefaultLink = GetFirstNode (&FormSet->DefaultStoreListHead);
> +        while (!IsNull (&FormSet->DefaultStoreListHead, DefaultLink)) {
> +          PreDefaultStore =
> FORMSET_DEFAULTSTORE_FROM_LINK(DefaultLink);
> +          DefaultLink = GetNextNode (&FormSet->DefaultStoreListHead,
> DefaultLink);
> +          if (DefaultStore->DefaultId < PreDefaultStore->DefaultId) {
> +            InsertTailList (&PreDefaultStore->Link, &DefaultStore->Link);
> +            HaveInserted = TRUE;
> +            break;
> +          }
> +        }
> +      }
> +      if (!HaveInserted) {
> +        InsertTailList (&FormSet->DefaultStoreListHead, &DefaultStore->Link);
> +      }
>        break;
> 
>      //
>      // Statements
>      //
> diff --git a/MdeModulePkg/Universal/SetupBrowserDxe/Setup.c
> b/MdeModulePkg/Universal/SetupBrowserDxe/Setup.c
> index 6b38547..66c4313 100644
> --- a/MdeModulePkg/Universal/SetupBrowserDxe/Setup.c
> +++ b/MdeModulePkg/Universal/SetupBrowserDxe/Setup.c
> @@ -4048,13 +4048,18 @@ GetQuestionDefault (
>    EFI_HII_CONFIG_ACCESS_PROTOCOL  *ConfigAccess;
>    EFI_BROWSER_ACTION_REQUEST      ActionRequest;
>    INTN                            Action;
>    CHAR16                          *NewString;
>    EFI_IFR_TYPE_VALUE              *TypeValue;
> +  UINT16                          OriginalDefaultId;
> +  FORMSET_DEFAULTSTORE            *DefaultStore;
> +  LIST_ENTRY                      *DefaultLink;
> 
>    Status   = EFI_NOT_FOUND;
>    StrValue = NULL;
> +  OriginalDefaultId  = DefaultId;
> +  DefaultLink        = GetFirstNode (&FormSet->DefaultStoreListHead);
> 
>    //
>    // Statement don't have storage, skip them
>    //
>    if (Question->QuestionId == 0) {
> @@ -4067,10 +4072,11 @@ GetQuestionDefault (
>    //  2, use ExtractConfig function
>    //  3, use nested EFI_IFR_DEFAULT
>    //  4, set flags of EFI_ONE_OF_OPTION (provide Standard and
> Manufacturing default)
>    //  5, set flags of EFI_IFR_CHECKBOX (provide Standard and Manufacturing
> default) (lowest priority)
>    //
> +ReGetDefault:
>    HiiValue = &Question->HiiValue;
>    TypeValue = &HiiValue->Value;
>    if (HiiValue->Type == EFI_IFR_TYPE_BUFFER) {
>      //
>      // For orderedlist, need to pass the BufferValue to Callback function.
> @@ -4233,11 +4239,26 @@ GetQuestionDefault (
>        return EFI_SUCCESS;
>      }
>    }
> 
>    //
> -  // For Questions without default
> +  // For question without default value for current default Id, we try to re-
> get the default value form other default id in the DefaultStoreList.
> +  // If get, will exit the function, if not, will choose next default id in the
> DefaultStoreList.
> +  // The default id in DefaultStoreList are in ascending order to make sure
> choose the smallest default id every time.
> +  //
> +  while (!IsNull(&FormSet->DefaultStoreListHead, DefaultLink)) {
> +    DefaultStore = FORMSET_DEFAULTSTORE_FROM_LINK(DefaultLink);
> +    DefaultLink = GetNextNode (&FormSet-
> >DefaultStoreListHead,DefaultLink);
> +    DefaultId = DefaultStore->DefaultId;
> +    if (DefaultId == OriginalDefaultId) {
> +      continue;
> +    }
> +    goto ReGetDefault;
> +  }
> +
> +  //
> +  // For Questions without default value for all the default id in the
> DefaultStoreList.
>    //
>    Status = EFI_NOT_FOUND;
>    switch (Question->Operand) {
>    case EFI_IFR_NUMERIC_OP:
>      //
> --
> 1.9.5.msysgit.1



^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2016-08-10 12:26 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-08-09  1:24 [PATCH v2 0/2] Share default value if some default value are not specified Dandan Bi
2016-08-09  1:24 ` [PATCH v2 1/2] MdeModulePkg/HiiDB: Share default " Dandan Bi
2016-08-10 10:02   ` Gao, Liming
2016-08-09  1:24 ` [PATCH v2 2/2] MdeModulePkg/Browser: " Dandan Bi
2016-08-10 12:26   ` Gao, Liming

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox