On 10/08/18 16:32, Bi, Dandan wrote: > Hi Laszlo, > >> -----Original Message----- >> From: Laszlo Ersek [mailto:ler...@redhat.com] >> Sent: Monday, October 08, 2018 8:19 PM >> To: Bi, Dandan <dandan...@intel.com>; edk2-devel@lists.01.org >> Cc: Zeng, Star <star.z...@intel.com>; Gao, Liming <liming....@intel.com> >> Subject: Re: [edk2] [patch] MdeModulePkg/HiiDB: Fix incorrect structure >> convention for checkbox >> >> Hi Dandan, >> >> On 10/08/18 03:29, Dandan Bi wrote: >>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1224 >>> >>> When covert IFR binary to EFI_IFR_CHECKBOX structure, Current code has >>> following incorrect code logic: >>> IfrCheckBox = (EFI_IFR_CHECKBOX *) (IfrOpHdr + 1); The correct one >>> should be: >>> IfrCheckBox = (EFI_IFR_CHECKBOX *) IfrOpHdr; >>> >>> This patch is to fix this bug. >>> >>> Cc: Liming Gao <liming....@intel.com> >>> Cc: Star Zeng <star.z...@intel.com> >>> Contributed-under: TianoCore Contribution Agreement 1.1 >>> Signed-off-by: Dandan Bi <dandan...@intel.com> >>> --- >>> MdeModulePkg/Universal/HiiDatabaseDxe/Database.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/MdeModulePkg/Universal/HiiDatabaseDxe/Database.c >>> b/MdeModulePkg/Universal/HiiDatabaseDxe/Database.c >>> index 45448c5198..664687796f 100644 >>> --- a/MdeModulePkg/Universal/HiiDatabaseDxe/Database.c >>> +++ b/MdeModulePkg/Universal/HiiDatabaseDxe/Database.c >>> @@ -896,11 +896,11 @@ UpdateDefaultSettingInFormPackage ( >>> break; >>> case EFI_IFR_CHECKBOX_OP: >>> IfrScope = IfrOpHdr->Scope; >>> IfrQuestionType = IfrOpHdr->OpCode; >>> IfrQuestionHdr = (EFI_IFR_QUESTION_HEADER *) (IfrOpHdr + 1); >>> - IfrCheckBox = (EFI_IFR_CHECKBOX *) (IfrOpHdr + 1); >>> + IfrCheckBox = (EFI_IFR_CHECKBOX *) IfrOpHdr; >>> EfiVarStoreIndex = IsEfiVarStoreQuestion (IfrQuestionHdr, >> EfiVarStoreList, EfiVarStoreNumber); >>> Width = sizeof (BOOLEAN); >>> if (EfiVarStoreIndex < EfiVarStoreNumber) { >>> for (Index = 0; Index < DefaultIdNumber; Index ++) { >>> if (DefaultIdList[Index] == EFI_HII_DEFAULT_CLASS_STANDARD) >>> { >>> >> >> what were the practical consequences (symptoms) of this issue? Did some >> checkboxes not work? (I'm asking because SecureBootConfigDxe uses some >> checkboxes.) > > 1. The bug is in function "UpdateDefaultSettingInFormPackage()" which is to > update the default setting of some HII Questions in the IFR binary data. So > it only has impact when platform overrides default setting in HII VarStore > through DynamicHii PCD setting in Platform DSC file. If platform doesn't > override default setting, it has no impact. > > 2. The implementation updates the "Flags" filed in the EFI_IFR_CHECKBOX > structure to update the default setting of checkbox. > If using "IfrCheckBox = (EFI_IFR_CHECKBOX *) (IfrOpHdr + 1);" when wants > to update the " Flags" filed in checkbox, but in fact it will update the > opcode binary data(opcode binary length) behind checkbox binary. > And then it will cause Browser can't parse the IFR binary data correctly. And > then the possible symptom is that some HII Question and forms may be not > parsed and then cannot be shown.
Thanks! I've copied this into the BZ. Laszlo _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
