Reviewed-by: Ruiyu Ni <ruiyu...@intel.com> Thanks/Ray
> -----Original Message----- > From: Bi, Dandan > Sent: Monday, February 27, 2017 3:15 PM > To: edk2-devel@lists.01.org > Cc: Dong, Eric <eric.d...@intel.com>; Ni, Ruiyu <ruiyu...@intel.com>; Zeng, > Star <star.z...@intel.com> > Subject: [patch] MdeModulePkg/BMMUiLib: Replace same logic with API in > UefiBootManagerLib > > Use the API　EfiBootManagerDeleteLoadOptionVariable in > UefiBootManagerLib　to replace the same logic in function > Var_DelBootOption/Var_DelDriverOption. > This can make code clean and prevent potential bugs. > > https://bugzilla.tianocore.org/show_bug.cgi?id=391 > > Cc: Eric Dong <eric.d...@intel.com> > Cc: Ruiyu Ni <ruiyu...@intel.com> > Cc: Star Zeng <star.z...@intel.com> > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Dandan Bi <dandan...@intel.com> > --- > .../BootMaintenanceManager.h | 37 +--- > .../Library/BootMaintenanceManagerUiLib/Variable.c | 202 +------------------- > - > 2 files changed, 11 insertions(+), 228 deletions(-) > > diff --git > a/MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenanc > eManager.h > b/MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenanc > eManager.h > index 532b75b..a8d7a0f 100644 > --- > a/MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenanc > eManager.h > +++ > b/MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenanc > eMa > +++ nager.h > @@ -631,12 +631,10 @@ Var_UpdateBootOption ( > IN BMM_CALLBACK_DATA *CallbackData > ); > > /** > Delete Boot Option that represent a Deleted state in BootOptionMenu. > - After deleting this boot option, call Var_ChangeBootOrder to > - make sure BootOrder is in valid state. > > @retval EFI_SUCCESS If all boot load option EFI Variables corresponding > to > BM_LOAD_CONTEXT marked for deletion is deleted > @return Others If failed to update the "BootOrder" variable after > deletion. > > @@ -645,25 +643,10 @@ EFI_STATUS > Var_DelBootOption ( > VOID > ); > > /** > - After any operation on Boot####, there will be a discrepancy in BootOrder. > - Since some are missing but in BootOrder, while some are present but are > - not reflected by BootOrder. Then a function rebuild BootOrder from > - scratch by content from BootOptionMenu is needed. > - > - @retval EFI_SUCCESS The boot order is updated successfully. > - @return other than EFI_SUCCESS if failed to change the "BootOrder" EFI > Variable. > - > -**/ > -EFI_STATUS > -Var_ChangeBootOrder ( > - VOID > - ); > - > -/** > This function create a currently loaded Drive Option from > the BMM. It then appends this Driver Option to the end of > the "DriverOrder" list. It append this Driver Opotion to the end > of DriverOptionMenu. > > @@ -685,13 +668,11 @@ Var_UpdateDriverOption ( > IN UINT16 *OptionalData, > IN UINT8 ForceReconnect > ); > > /** > - Delete Load Option that represent a Deleted state in BootOptionMenu. > - After deleting this Driver option, call Var_ChangeDriverOrder to > - make sure DriverOrder is in valid state. > + Delete Load Option that represent a Deleted state in DriverOptionMenu. > > @retval EFI_SUCCESS Load Option is successfully updated. > @return Other value than EFI_SUCCESS if failed to update "Driver Order" > EFI > Variable. > > @@ -700,26 +681,10 @@ EFI_STATUS > Var_DelDriverOption ( > VOID > ); > > /** > - After any operation on Driver####, there will be a discrepancy in > - DriverOrder. Since some are missing but in DriverOrder, while some > - are present but are not reflected by DriverOrder. Then a function > - rebuild DriverOrder from scratch by content from DriverOptionMenu is > - needed. > - > - @retval EFI_SUCCESS The driver order is updated successfully. > - @return other than EFI_SUCCESS if failed to set the "DriverOrder" EFI > Variable. > - > -**/ > -EFI_STATUS > -Var_ChangeDriverOrder ( > - VOID > - ); > - > -/** > This function delete and build multi-instance device path ConIn > console device. > > @retval EFI_SUCCESS The function complete successfully. > @return The EFI variable can not be saved. See gRT->SetVariable for detail > return information. > diff --git > a/MdeModulePkg/Library/BootMaintenanceManagerUiLib/Variable.c > b/MdeModulePkg/Library/BootMaintenanceManagerUiLib/Variable.c > index 746b233..c1c55b0 100644 > --- a/MdeModulePkg/Library/BootMaintenanceManagerUiLib/Variable.c > +++ b/MdeModulePkg/Library/BootMaintenanceManagerUiLib/Variable.c > @@ -1,9 +1,9 @@ > /** @file > Variable operation that will be used by bootmaint > > -Copyright (c) 2004 - 2016, Intel Corporation. All rights reserved.<BR> > +Copyright (c) 2004 - 2017, 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 > > @@ -14,12 +14,10 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF > ANY KIND, EITHER EXPRESS OR IMPLIED. > > #include "BootMaintenanceManager.h" > > /** > Delete Boot Option that represent a Deleted state in BootOptionMenu. > - After deleting this boot option, call Var_ChangeBootOrder to > - make sure BootOrder is in valid state. > > @retval EFI_SUCCESS If all boot load option EFI Variables corresponding > to > BM_LOAD_CONTEXT marked for deletion is deleted. > @retval EFI_NOT_FOUND If can not find the boot option want to be > deleted. > @return Others If failed to update the "BootOrder" variable after > deletion. > @@ -30,16 +28,14 @@ Var_DelBootOption ( > VOID > ) > { > BM_MENU_ENTRY *NewMenuEntry; > BM_LOAD_CONTEXT *NewLoadContext; > - UINT16 BootString[10]; > EFI_STATUS Status; > UINTN Index; > UINTN Index2; > > - Status = EFI_SUCCESS; > Index2 = 0; > for (Index = 0; Index < BootOptionMenu.MenuNumber; Index++) { > NewMenuEntry = BOpt_GetMenuEntry (&BootOptionMenu, (Index - > Index2)); > if (NULL == NewMenuEntry) { > return EFI_NOT_FOUND; > @@ -48,18 +44,14 @@ Var_DelBootOption ( > NewLoadContext = (BM_LOAD_CONTEXT *) NewMenuEntry- > >VariableContext; > if (!NewLoadContext->Deleted) { > continue; > } > > - UnicodeSPrint ( > - BootString, > - sizeof (BootString), > - L"Boot%04x", > - NewMenuEntry->OptionNumber > - ); > - > - EfiLibDeleteVariable (BootString, &gEfiGlobalVariableGuid); > + Status = EfiBootManagerDeleteLoadOptionVariable (NewMenuEntry- > >OptionNumber,LoadOptionTypeBoot); > + if (EFI_ERROR (Status)) { > + return Status; > + } > Index2++; > // > // If current Load Option is the same as BootNext, > // must delete BootNext in order to make sure > // there will be no panic on next boot @@ -73,106 +65,15 @@ > Var_DelBootOption ( > NewMenuEntry = NULL; > } > > BootOptionMenu.MenuNumber -= Index2; > > - Status = Var_ChangeBootOrder (); > - return Status; > -} > - > -/** > - After any operation on Boot####, there will be a discrepancy in BootOrder. > - Since some are missing but in BootOrder, while some are present but are > - not reflected by BootOrder. Then a function rebuild BootOrder from > - scratch by content from BootOptionMenu is needed. > - > - > - > - > - @retval EFI_SUCCESS The boot order is updated successfully. > - @return EFI_STATUS other than EFI_SUCCESS if failed to > - Set the "BootOrder" EFI Variable. > - > -**/ > -EFI_STATUS > -Var_ChangeBootOrder ( > - VOID > - ) > -{ > - > - EFI_STATUS Status; > - BM_MENU_ENTRY *NewMenuEntry; > - UINT16 *BootOrderList; > - UINT16 *BootOrderListPtr; > - UINTN BootOrderListSize; > - UINTN Index; > - > - BootOrderList = NULL; > - BootOrderListSize = 0; > - // > - // First check whether BootOrder is present in current configuration > - // > - GetEfiGlobalVariable2 (L"BootOrder", (VOID **) &BootOrderList, > &BootOrderListSize); > - > - // > - // If exists, delete it to hold new BootOrder > - // > - if (BootOrderList != NULL) { > - EfiLibDeleteVariable (L"BootOrder", &gEfiGlobalVariableGuid); > - FreePool (BootOrderList); > - BootOrderList = NULL; > - } > - // > - // Maybe here should be some check method to ensure that > - // no new added boot options will be added > - // but the setup engine now will give only one callback > - // that is to say, user are granted only one chance to > - // decide whether the boot option will be added or not > - // there should be no indictor to show whether this > - // is a "new" boot option > - // > - BootOrderListSize = BootOptionMenu.MenuNumber; > - > - if (BootOrderListSize > 0) { > - BootOrderList = AllocateZeroPool (BootOrderListSize * sizeof (UINT16)); > - ASSERT (BootOrderList != NULL); > - BootOrderListPtr = BootOrderList; > - > - // > - // Get all current used Boot#### from BootOptionMenu. > - // OptionNumber in each BM_LOAD_OPTION is really its > - // #### value. > - // > - for (Index = 0; Index < BootOrderListSize; Index++) { > - NewMenuEntry = BOpt_GetMenuEntry (&BootOptionMenu, Index); > - *BootOrderList = (UINT16) NewMenuEntry->OptionNumber; > - BootOrderList++; > - } > - > - BootOrderList = BootOrderListPtr; > - > - // > - // After building the BootOrderList, write it back > - // > - Status = gRT->SetVariable ( > - L"BootOrder", > - &gEfiGlobalVariableGuid, > - VAR_FLAG, > - BootOrderListSize * sizeof (UINT16), > - BootOrderList > - ); > - if (EFI_ERROR (Status)) { > - return Status; > - } > - } > return EFI_SUCCESS; > } > > /** > - Delete Load Option that represent a Deleted state in BootOptionMenu. > - After deleting this Driver option, call Var_ChangeDriverOrder to > - make sure DriverOrder is in valid state. > + Delete Load Option that represent a Deleted state in DriverOptionMenu. > > @retval EFI_SUCCESS Load Option is successfully updated. > @retval EFI_NOT_FOUND Fail to find the driver option want to be > deleted. > @return Other value than EFI_SUCCESS if failed to update "Driver Order" > EFI > Variable. > @@ -183,16 +84,14 @@ Var_DelDriverOption ( > VOID > ) > { > BM_MENU_ENTRY *NewMenuEntry; > BM_LOAD_CONTEXT *NewLoadContext; > - UINT16 DriverString[12]; > EFI_STATUS Status; > UINTN Index; > UINTN Index2; > > - Status = EFI_SUCCESS; > Index2 = 0; > for (Index = 0; Index < DriverOptionMenu.MenuNumber; Index++) { > NewMenuEntry = BOpt_GetMenuEntry (&DriverOptionMenu, (Index - > Index2)); > if (NULL == NewMenuEntry) { > return EFI_NOT_FOUND; > @@ -200,105 +99,24 @@ Var_DelDriverOption ( > > NewLoadContext = (BM_LOAD_CONTEXT *) NewMenuEntry- > >VariableContext; > if (!NewLoadContext->Deleted) { > continue; > } > + Status = EfiBootManagerDeleteLoadOptionVariable (NewMenuEntry- > >OptionNumber,LoadOptionTypeDriver); > + if (EFI_ERROR (Status)) { > + return Status; > + } > > - UnicodeSPrint ( > - DriverString, > - sizeof (DriverString), > - L"Driver%04x", > - NewMenuEntry->OptionNumber > - ); > - > - EfiLibDeleteVariable (DriverString, &gEfiGlobalVariableGuid); > Index2++; > > RemoveEntryList (&NewMenuEntry->Link); > BOpt_DestroyMenuEntry (NewMenuEntry); > NewMenuEntry = NULL; > } > > DriverOptionMenu.MenuNumber -= Index2; > > - Status = Var_ChangeDriverOrder (); > - return Status; > -} > - > -/** > - After any operation on Driver####, there will be a discrepancy in > - DriverOrder. Since some are missing but in DriverOrder, while some > - are present but are not reflected by DriverOrder. Then a function > - rebuild DriverOrder from scratch by content from DriverOptionMenu is > - needed. > - > - @retval EFI_SUCCESS The driver order is updated successfully. > - @return Other status than EFI_SUCCESS if failed to set the "DriverOrder" > EFI Variable. > - > -**/ > -EFI_STATUS > -Var_ChangeDriverOrder ( > - VOID > - ) > -{ > - EFI_STATUS Status; > - BM_MENU_ENTRY *NewMenuEntry; > - UINT16 *DriverOrderList; > - UINT16 *DriverOrderListPtr; > - UINTN DriverOrderListSize; > - UINTN Index; > - > - DriverOrderList = NULL; > - DriverOrderListSize = 0; > - > - // > - // First check whether DriverOrder is present in current configuration > - // > - GetEfiGlobalVariable2 (L"DriverOrder", (VOID **) &DriverOrderList, > &DriverOrderListSize); > - // > - // If exists, delete it to hold new DriverOrder > - // > - if (DriverOrderList != NULL) { > - EfiLibDeleteVariable (L"DriverOrder", &gEfiGlobalVariableGuid); > - FreePool (DriverOrderList); > - DriverOrderList = NULL; > - } > - > - DriverOrderListSize = DriverOptionMenu.MenuNumber; > - > - if (DriverOrderListSize > 0) { > - DriverOrderList = AllocateZeroPool (DriverOrderListSize * sizeof > (UINT16)); > - ASSERT (DriverOrderList != NULL); > - DriverOrderListPtr = DriverOrderList; > - > - // > - // Get all current used Driver#### from DriverOptionMenu. > - // OptionNumber in each BM_LOAD_OPTION is really its > - // #### value. > - // > - for (Index = 0; Index < DriverOrderListSize; Index++) { > - NewMenuEntry = BOpt_GetMenuEntry (&DriverOptionMenu, Index); > - *DriverOrderList = (UINT16) NewMenuEntry->OptionNumber; > - DriverOrderList++; > - } > - > - DriverOrderList = DriverOrderListPtr; > - > - // > - // After building the DriverOrderList, write it back > - // > - Status = gRT->SetVariable ( > - L"DriverOrder", > - &gEfiGlobalVariableGuid, > - VAR_FLAG, > - DriverOrderListSize * sizeof (UINT16), > - DriverOrderList > - ); > - if (EFI_ERROR (Status)) { > - return Status; > - } > - } > return EFI_SUCCESS; > } > > /** > This function delete and build multi-instance device path for > -- > 1.9.5.msysgit.1 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
