From: "Gao, Liming" <liming.gao@intel.com>
To: "Bi, Dandan" <dandan.bi@intel.com>,
"edk2-devel@lists.01.org" <edk2-devel@lists.01.org>
Cc: "Dong, Eric" <eric.dong@intel.com>
Subject: Re: [PATCH v2 1/2] MdeModulePkg/HiiDB: Share default if some default value are not specified
Date: Wed, 10 Aug 2016 10:02:25 +0000 [thread overview]
Message-ID: <4A89E2EF3DFEDB4C8BFDE51014F606A1155E972A@shsmsx102.ccr.corp.intel.com> (raw)
In-Reply-To: <1470705898-99100-2-git-send-email-dandan.bi@intel.com>
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
next prev parent reply other threads:[~2016-08-10 10:02 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
2016-08-09 1:24 ` [PATCH v2 2/2] MdeModulePkg/Browser: " Dandan Bi
2016-08-10 12:26 ` Gao, Liming
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4A89E2EF3DFEDB4C8BFDE51014F606A1155E972A@shsmsx102.ccr.corp.intel.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox