Re: [PATCH] Fix Setup numeric default value incorrect issue

["Chen Lin Z" <lin.z.chen@intel.com>]

OK, I'll update patch.

Thanks,
Lin

-----Original Message-----
From: gaoliming <gaoliming@byosoft.com.cn> 
Sent: Friday, April 1, 2022 10:01 AM
To: Chen, Lin Z <lin.z.chen@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; Bi, Dandan <dandan.bi@intel.com>; Dong, Eric <eric.dong@intel.com>; devel@edk2.groups.io
Cc: Li, Zhuangzhi <zhuangzhi.li@intel.com>; Zhang, Di <di.zhang@intel.com>
Subject: 回复: [PATCH] Fix Setup numeric default value incorrect issue

Lin:
 Thanks for your fix. This is the real issue for numeric opcode. I have one comment here. The allocated IfrEfiVarStoreTmp is required to be free. 

Thanks
Liming
> -----邮件原件-----
> 发件人: Chen, Lin Z <lin.z.chen@intel.com>
> 发送时间: 2022年3月31日 15:29
> 收件人: Wang, Jian J <jian.j.wang@intel.com>; Gao, Liming 
> <gaoliming@byosoft.com.cn>; Bi, Dandan <dandan.bi@intel.com>; Dong, 
> Eric <eric.dong@intel.com>; devel@edk2.groups.io
> 抄送: Li, Zhuangzhi <zhuangzhi.li@intel.com>; Zhang, Di
<di.zhang@intel.com>
> 主题: RE: [PATCH] Fix Setup numeric default value incorrect issue
> 
> Hi All,
> 
>   Any comments about patch ?
> 
> Thanks,
> Lin
> 
> -----Original Message-----
> From: Chen, Lin Z <lin.z.chen@intel.com>
> Sent: Monday, March 28, 2022 9:27 PM
> To: Wang, Jian J <jian.j.wang@intel.com>; Gao, Liming 
> <gaoliming@byosoft.com.cn>; Bi, Dandan <dandan.bi@intel.com>; Dong, 
> Eric <eric.dong@intel.com>; devel@edk2.groups.io
> Cc: Li, Zhuangzhi <zhuangzhi.li@intel.com>; Zhang, Di
<di.zhang@intel.com>;
> Chen, Lin Z <lin.z.chen@intel.com>
> Subject: [PATCH] Fix Setup numeric default value incorrect issue
> 
> When default/manufacturing flag get removed from numeric varid, it 
> can't
get
> default value from StructurePcd in 'UpdateDefaultSettingInFormPackage'
> function since there is no EFI_IFR_DEFAULT_OP opcode in IFR file. Add 
> a chance to get numeric default value from StructurePcd in the case 
> that numeric minimum value will be used as default value.
> 
> Signed-off-by: Chen Lin Z <lin.z.chen@intel.com>
> Signed-off-by: Dandan Bi <dandan.bi@intel.com>
> ---
>  .../Universal/HiiDatabaseDxe/ConfigRouting.c  | 14 +++++++++++
>  .../Universal/HiiDatabaseDxe/HiiDatabase.h    | 23
> +++++++++++++++++++
>  2 files changed, 37 insertions(+)
> 
> diff --git a/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c
> b/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c
> index 2f792d2965..8bfa0f4bf1 100644
> --- a/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c
> +++ b/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c
> @@ -2171,6 +2171,7 @@ ParseIfrData (
>    UINTN                        PackageOffset;
> EFI_IFR_VARSTORE             *IfrVarStore;   EFI_IFR_VARSTORE_EFI
> *IfrEfiVarStore;+  EFI_IFR_VARSTORE_EFI         *IfrEfiVarStoreTmp;
> EFI_IFR_OP_HEADER            *IfrOpHdr;   EFI_IFR_ONE_OF
> *IfrOneOf;   EFI_IFR_REF4                 *IfrRef;@@ -2187,6
> +2188,7 @@ ParseIfrData (
>    IFR_BLOCK_DATA               *BlockData;   CHAR16
> *VarStoreName;   UINTN                        NameSize;+
> UINTN                        NvDefaultStoreSize;   UINT16
> VarWidth;   UINT16                       VarDefaultId;   BOOLEAN
> FirstOneOfOption;@@ -2303,6 +2305,14 @@ ParseIfrData (
>          }          AsciiStrToUnicodeStrS ((CHAR8
> *)IfrEfiVarStore->Name, VarStoreName, NameSize);+ IfrEfiVarStoreTmp = 
> AllocatePool (IfrEfiVarStore->Header.Length +
AsciiStrSize
> ((CHAR8 *)IfrEfiVarStore->Name));+        if (IfrEfiVarStoreTmp == NULL)
> {+          Status = EFI_OUT_OF_RESOURCES;+          goto
> Done;+        }++        CopyMem (IfrEfiVarStoreTmp, IfrEfiVarStore,
> IfrEfiVarStore->Header.Length);+        AsciiStrToUnicodeStrS ((CHAR8
> *)IfrEfiVarStore->Name, (CHAR16 *)&(IfrEfiVarStoreTmp->Name[0]), 
> AsciiStrSize ((CHAR8 *)IfrEfiVarStore->Name) * sizeof (CHAR16)); if 
> (IsThisVarstore (&IfrEfiVarStore->Guid, VarStoreName, ConfigHdr))
> {           //@@ -2502,9 +2512,13 @@ ParseIfrData (
>            //           // Set default value base on the DefaultId list
> get from IFR data.           //+          NvDefaultStoreSize =
> PcdGetSize (PcdNvStoreDefaultValueBuffer);           for (LinkData =
> DefaultIdArray->Entry.ForwardLink; LinkData != &DefaultIdArray->Entry;
> LinkData = LinkData->ForwardLink) {             DefaultDataPtr
> = BASE_CR (LinkData, IFR_DEFAULT_DATA, Entry);
> DefaultData.DefaultId = DefaultDataPtr->DefaultId;+            if
> (NvDefaultStoreSize > sizeof (PCD_NV_STORE_DEFAULT_BUFFER_HEADER))
> {+              FindQuestionDefaultSetting (DefaultData.DefaultId,
> IfrEfiVarStoreTmp, &(IfrOneOf->Question), &DefaultData.Value, VarWidth,
> QuestionReferBitField);+            }             InsertDefaultValue
> (BlockData, &DefaultData);           }         }diff --git
> a/MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabase.h
> b/MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabase.h
> index c4ca6ad6ee..421c293cfc 100644
> --- a/MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabase.h
> +++ b/MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabase.h
> @@ -2308,6 +2308,29 @@ HiiGetConfigRespInfo (
>    IN CONST EFI_HII_DATABASE_PROTOCOL  *This   ); +/**+  Find
> question default value from PcdNvStoreDefaultValueBuffer++  @param
> DefaultId          Default store ID+  @param EfiVarStore        Point
> to EFI VarStore header+  @param IfrQuestionHdr     Point to Question
> header+  @param ValueBuffer        Point to Buffer includes the found
> default setting+  @param Width              Width of the default
> value+  @param BitFieldQuestion   Whether the Question is stored in Bit
> field.++  @retval EFI_SUCCESS       Question default value is found.+
> @retval EFI_NOT_FOUND     Question default value is not
> found.+**/+EFI_STATUS+FindQuestionDefaultSetting (+  IN  UINT16
> DefaultId,+  IN  EFI_IFR_VARSTORE_EFI     *EfiVarStore,+  IN
> EFI_IFR_QUESTION_HEADER  *IfrQuestionHdr,+  OUT VOID
> *ValueBuffer,+  IN  UINTN                    Width,+  IN
> BOOLEAN                  BitFieldQuestion+  );+ // // Global variables
> //--
> 2.25.1