Reviewed-by: Eric Dong <eric.d...@intel.com> > -----Original Message----- > From: Bi, Dandan > Sent: Wednesday, February 17, 2016 6:15 PM > To: edk2-devel@lists.01.org > Cc: Gao, Liming; Dong, Eric > Subject: [patch] MdeModulePkg: Refine the code in BootMaintenanceManagerUiLib > > Refine the code in function Var_UpdateDriverOption and Var_UpdateBootOption, > use the existed API(EfiBootManagerInitializeLoadOption and > EfiBootManagerAddLoadOptionVariable) supplied by UefiBootManagerLib > to replace the same logic in the two functions. And remove the useless > code. > > Cc: Liming Gao <liming....@intel.com> > Cc: Eric Dong <eric.d...@intel.com> > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Dandan Bi <dandan...@intel.com> > --- > .../BootMaintenanceManager.h | 11 +- > .../BootMaintenanceManagerUiLib/BootOption.c | 21 +- > .../Library/BootMaintenanceManagerUiLib/Variable.c | 226 > ++++++--------------- > 3 files changed, 71 insertions(+), 187 deletions(-) > > diff --git > a/MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenanceManager.h > b/MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenanceManager.h > index 24526e1..b482bf2 100644 > --- > a/MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenanceManager.h > +++ > b/MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenanceManager.h > @@ -1,9 +1,9 @@ > /** @file > Header file for boot maintenance module. > > -Copyright (c) 2004 - 2015, Intel Corporation. All rights reserved.<BR> > +Copyright (c) 2004 - 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 > > @@ -266,28 +266,19 @@ typedef struct { > EFI_DEVICE_PATH_PROTOCOL *DevicePath; > } BM_TERMINAL_CONTEXT; > > typedef struct { > BOOLEAN IsBootNext; > - BOOLEAN LoadOptionModified; > BOOLEAN Deleted; > > BOOLEAN IsLegacy; > - BOOLEAN IsActive; > - BOOLEAN ForceReconnect; > - UINTN OptionalDataSize; > - > - UINTN LoadOptionSize; > - UINT8 *LoadOption; > > UINT32 Attributes; > UINT16 FilePathListLength; > UINT16 *Description; > EFI_DEVICE_PATH_PROTOCOL *FilePathList; > UINT8 *OptionalData; > - > - UINT16 BbsIndex; > } BM_LOAD_CONTEXT; > > typedef struct { > > BOOLEAN IsActive; > diff --git a/MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootOption.c > b/MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootOption.c > index a375d61..5665d04 100644 > --- a/MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootOption.c > +++ b/MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootOption.c > @@ -3,11 +3,11 @@ > > Include file system navigation, system handle selection > > Boot option manipulation > > -Copyright (c) 2004 - 2015, Intel Corporation. All rights reserved.<BR> > +Copyright (c) 2004 - 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 > > @@ -114,11 +114,10 @@ BOpt_DestroyMenuEntry ( > // > switch (MenuEntry->ContextSelection) { > case BM_LOAD_CONTEXT_SELECT: > LoadContext = (BM_LOAD_CONTEXT *) MenuEntry->VariableContext; > FreePool (LoadContext->FilePathList); > - FreePool (LoadContext->LoadOption); > if (LoadContext->OptionalData != NULL) { > FreePool (LoadContext->OptionalData); > } > FreePool (LoadContext); > break; > @@ -334,11 +333,10 @@ BOpt_GetBootOptions ( > > LoadOptionPtr = LoadOption; > LoadOptionEnd = LoadOption + BootOptionSize; > > NewMenuEntry->OptionNumber = BootOrderList[Index]; > - NewLoadContext->LoadOptionModified = FALSE; > NewLoadContext->Deleted = FALSE; > NewLoadContext->IsBootNext = BootNextFlag; > > // > // Is a Legacy Device? > @@ -372,17 +370,12 @@ BOpt_GetBootOptions ( > // > // LoadOption is a pointer type of UINT8 > // for easy use with following LOAD_OPTION > // embedded in this struct > // > - NewLoadContext->LoadOption = LoadOption; > - NewLoadContext->LoadOptionSize = BootOptionSize; > > NewLoadContext->Attributes = *(UINT32 *) LoadOptionPtr; > - NewLoadContext->IsActive = (BOOLEAN) (NewLoadContext->Attributes > & LOAD_OPTION_ACTIVE); > - > - NewLoadContext->ForceReconnect = (BOOLEAN) (NewLoadContext->Attributes > & LOAD_OPTION_FORCE_RECONNECT); > > LoadOptionPtr += sizeof (UINT32); > > NewLoadContext->FilePathListLength = *(UINT16 *) LoadOptionPtr; > LoadOptionPtr += sizeof (UINT16); > @@ -424,12 +417,10 @@ BOpt_GetBootOptions ( > CopyMem ( > NewLoadContext->OptionalData, > LoadOptionPtr, > OptionalDataSize > ); > - > - NewLoadContext->OptionalDataSize = OptionalDataSize; > } > > InsertTailList (&BootOptionMenu.Head, &NewMenuEntry->Link); > MenuCount++; > } > @@ -439,10 +430,12 @@ BOpt_GetBootOptions ( > FreePool (BootNext); > } > if (BootOrderList != NULL) { > FreePool (BootOrderList); > } > + > + FreePool(LoadOption); > BootOptionMenu.MenuNumber = MenuCount; > return EFI_SUCCESS; > } > > /** > @@ -706,26 +699,20 @@ BOpt_GetDriverOptions ( > > NewLoadContext = (BM_LOAD_CONTEXT *) > NewMenuEntry->VariableContext; > LoadOptionPtr = LoadOption; > LoadOptionEnd = LoadOption + DriverOptionSize; > NewMenuEntry->OptionNumber = DriverOrderList[Index]; > - NewLoadContext->LoadOptionModified = FALSE; > NewLoadContext->Deleted = FALSE; > NewLoadContext->IsLegacy = FALSE; > > // > // LoadOption is a pointer type of UINT8 > // for easy use with following LOAD_OPTION > // embedded in this struct > // > - NewLoadContext->LoadOption = LoadOption; > - NewLoadContext->LoadOptionSize = DriverOptionSize; > > NewLoadContext->Attributes = *(UINT32 *) LoadOptionPtr; > - NewLoadContext->IsActive = (BOOLEAN) (NewLoadContext->Attributes > & LOAD_OPTION_ACTIVE); > - > - NewLoadContext->ForceReconnect = (BOOLEAN) (NewLoadContext->Attributes > & LOAD_OPTION_FORCE_RECONNECT); > > LoadOptionPtr += sizeof (UINT32); > > NewLoadContext->FilePathListLength = *(UINT16 *) LoadOptionPtr; > LoadOptionPtr += sizeof (UINT16); > @@ -769,20 +756,20 @@ BOpt_GetDriverOptions ( > NewLoadContext->OptionalData, > LoadOptionPtr, > OptionalDataSize > ); > > - NewLoadContext->OptionalDataSize = OptionalDataSize; > } > > InsertTailList (&DriverOptionMenu.Head, &NewMenuEntry->Link); > > } > > if (DriverOrderList != NULL) { > FreePool (DriverOrderList); > } > + FreePool(LoadOption); > DriverOptionMenu.MenuNumber = Index; > return EFI_SUCCESS; > > } > > diff --git a/MdeModulePkg/Library/BootMaintenanceManagerUiLib/Variable.c > b/MdeModulePkg/Library/BootMaintenanceManagerUiLib/Variable.c > index a276cae..929f383 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 - 2015, Intel Corporation. All rights reserved.<BR> > +Copyright (c) 2004 - 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 > > @@ -533,23 +533,22 @@ Var_UpdateDriverOption ( > IN UINT16 *OptionalData, > IN UINT8 ForceReconnect > ) > { > UINT16 Index; > - UINT16 *DriverOrderList; > - UINT16 *NewDriverOrderList; > UINT16 DriverString[12]; > - UINTN DriverOrderListSize; > - VOID *Buffer; > - UINTN BufferSize; > - UINT8 *Ptr; > BM_MENU_ENTRY *NewMenuEntry; > BM_LOAD_CONTEXT *NewLoadContext; > BOOLEAN OptionalDataExist; > EFI_STATUS Status; > + EFI_BOOT_MANAGER_LOAD_OPTION LoadOption; > + UINT8 *OptionalDesData; > + UINT32 OptionalDataSize; > > OptionalDataExist = FALSE; > + OptionalDesData = NULL; > + OptionalDataSize = 0; > > Index = BOpt_GetDriverOptionNumber (); > UnicodeSPrint ( > DriverString, > sizeof (DriverString), > @@ -559,125 +558,76 @@ Var_UpdateDriverOption ( > > if (*DescriptionData == 0x0000) { > StrCpyS (DescriptionData, MAX_MENU_NUMBER, DriverString); > } > > - BufferSize = sizeof (UINT32) + sizeof (UINT16) + StrSize (DescriptionData); > - BufferSize += GetDevicePathSize (CallbackData->LoadContext->FilePathList); > - > if (*OptionalData != 0x0000) { > OptionalDataExist = TRUE; > - BufferSize += StrSize (OptionalData); > - } > - > - Buffer = AllocateZeroPool (BufferSize); > - if (NULL == Buffer) { > - return EFI_OUT_OF_RESOURCES; > + OptionalDesData = (UINT8 *)OptionalData; > + OptionalDataSize = (UINT32)StrSize (OptionalData); > } > > NewMenuEntry = BOpt_CreateMenuEntry (BM_LOAD_CONTEXT_SELECT); > if (NULL == NewMenuEntry) { > - FreePool (Buffer); > return EFI_OUT_OF_RESOURCES; > } > > + Status = EfiBootManagerInitializeLoadOption ( > + &LoadOption, > + Index, > + LoadOptionTypeDriver, > + LOAD_OPTION_ACTIVE | (ForceReconnect << 1), > + DescriptionData, > + CallbackData->LoadContext->FilePathList, > + OptionalDesData, > + OptionalDataSize > + ); > + if (!EFI_ERROR (Status)){ > + Status = EfiBootManagerAddLoadOptionVariable (&LoadOption,(UINTN) -1 ); > + } > + > NewLoadContext = (BM_LOAD_CONTEXT *) > NewMenuEntry->VariableContext; > NewLoadContext->Deleted = FALSE; > - NewLoadContext->LoadOptionSize = BufferSize; > - Ptr = (UINT8 *) Buffer; > - NewLoadContext->LoadOption = Ptr; > - *((UINT32 *) Ptr) = LOAD_OPTION_ACTIVE | (ForceReconnect << 1); > - NewLoadContext->Attributes = *((UINT32 *) Ptr); > - NewLoadContext->IsActive = TRUE; > - NewLoadContext->ForceReconnect = (BOOLEAN) (NewLoadContext->Attributes & > LOAD_OPTION_FORCE_RECONNECT); > - > - Ptr += sizeof (UINT32); > - *((UINT16 *) Ptr) = (UINT16) GetDevicePathSize > (CallbackData->LoadContext->FilePathList); > - NewLoadContext->FilePathListLength = *((UINT16 *) Ptr); > - > - Ptr += sizeof (UINT16); > - CopyMem ( > - Ptr, > - DescriptionData, > - StrSize (DescriptionData) > - ); > + NewLoadContext->Attributes = LoadOption.Attributes; > + NewLoadContext->FilePathListLength = (UINT16)GetDevicePathSize > (LoadOption.FilePath); > > NewLoadContext->Description = AllocateZeroPool (StrSize (DescriptionData)); > ASSERT (NewLoadContext->Description != NULL); > NewMenuEntry->DisplayString = NewLoadContext->Description; > CopyMem ( > NewLoadContext->Description, > - (VOID *) Ptr, > + LoadOption.Description, > StrSize (DescriptionData) > ); > > - Ptr += StrSize (DescriptionData); > - CopyMem ( > - Ptr, > - CallbackData->LoadContext->FilePathList, > - GetDevicePathSize (CallbackData->LoadContext->FilePathList) > - ); > - > NewLoadContext->FilePathList = AllocateZeroPool (GetDevicePathSize > (CallbackData->LoadContext->FilePathList)); > ASSERT (NewLoadContext->FilePathList != NULL); > - > CopyMem ( > NewLoadContext->FilePathList, > - (VOID *) Ptr, > + LoadOption.FilePath, > GetDevicePathSize (CallbackData->LoadContext->FilePathList) > ); > > NewMenuEntry->HelpString = UiDevicePathToStr > (NewLoadContext->FilePathList); > NewMenuEntry->OptionNumber = Index; > NewMenuEntry->DisplayStringToken = HiiSetString (HiiHandle, 0, > NewMenuEntry->DisplayString, NULL); > NewMenuEntry->HelpStringToken = HiiSetString (HiiHandle, 0, > NewMenuEntry->HelpString, NULL); > > if (OptionalDataExist) { > - Ptr += (UINT8) GetDevicePathSize > (CallbackData->LoadContext->FilePathList); > - > + NewLoadContext->OptionalData = AllocateZeroPool > (LoadOption.OptionalDataSize); > CopyMem ( > - Ptr, > - OptionalData, > - StrSize (OptionalData) > + NewLoadContext->OptionalData, > + LoadOption.OptionalData, > + LoadOption.OptionalDataSize > ); > } > > - Status = gRT->SetVariable ( > - DriverString, > - &gEfiGlobalVariableGuid, > - VAR_FLAG, > - BufferSize, > - Buffer > - ); > - ASSERT_EFI_ERROR (Status); > - GetEfiGlobalVariable2 (L"DriverOrder", (VOID **) &DriverOrderList, > &DriverOrderListSize); > - NewDriverOrderList = AllocateZeroPool (DriverOrderListSize + sizeof > (UINT16)); > - ASSERT (NewDriverOrderList != NULL); > - if (DriverOrderList != NULL){ > - CopyMem (NewDriverOrderList, DriverOrderList, DriverOrderListSize); > - } > - NewDriverOrderList[DriverOrderListSize / sizeof (UINT16)] = Index; > - if (DriverOrderList != NULL) { > - EfiLibDeleteVariable (L"DriverOrder", &gEfiGlobalVariableGuid); > - } > - > - Status = gRT->SetVariable ( > - L"DriverOrder", > - &gEfiGlobalVariableGuid, > - VAR_FLAG, > - DriverOrderListSize + sizeof (UINT16), > - NewDriverOrderList > - ); > - ASSERT_EFI_ERROR (Status); > - if (DriverOrderList != NULL) { > - FreePool (DriverOrderList); > - } > - DriverOrderList = NULL; > - FreePool (NewDriverOrderList); > InsertTailList (&DriverOptionMenu.Head, &NewMenuEntry->Link); > DriverOptionMenu.MenuNumber++; > > + EfiBootManagerFreeLoadOption(&LoadOption); > + > return EFI_SUCCESS; > } > > /** > This function create a currently loaded Boot Option from > @@ -694,145 +644,101 @@ Var_UpdateDriverOption ( > EFI_STATUS > Var_UpdateBootOption ( > IN BMM_CALLBACK_DATA *CallbackData > ) > { > - UINT16 *BootOrderList; > - UINT16 *NewBootOrderList; > - UINTN BootOrderListSize; > UINT16 BootString[10]; > - VOID *Buffer; > - UINTN BufferSize; > - UINT8 *Ptr; > UINT16 Index; > BM_MENU_ENTRY *NewMenuEntry; > BM_LOAD_CONTEXT *NewLoadContext; > BOOLEAN OptionalDataExist; > EFI_STATUS Status; > BMM_FAKE_NV_DATA *NvRamMap; > + EFI_BOOT_MANAGER_LOAD_OPTION LoadOption; > + UINT8 *OptionalData; > + UINT32 OptionalDataSize; > > OptionalDataExist = FALSE; > NvRamMap = &CallbackData->BmmFakeNvData; > + OptionalData = NULL; > + OptionalDataSize = 0; > > Index = BOpt_GetBootOptionNumber () ; > UnicodeSPrint (BootString, sizeof (BootString), L"Boot%04x", Index); > > if (NvRamMap->BootDescriptionData[0] == 0x0000) { > StrCpyS (NvRamMap->BootDescriptionData, sizeof > (NvRamMap->BootDescriptionData) / sizeof (NvRamMap->BootDescriptionData[0]), > BootString); > } > > - BufferSize = sizeof (UINT32) + sizeof (UINT16) + StrSize > (NvRamMap->BootDescriptionData); > - BufferSize += GetDevicePathSize (CallbackData->LoadContext->FilePathList); > - > if (NvRamMap->BootOptionalData[0] != 0x0000) { > OptionalDataExist = TRUE; > - BufferSize += StrSize (NvRamMap->BootOptionalData); > - } > - > - Buffer = AllocateZeroPool (BufferSize); > - if (NULL == Buffer) { > - return EFI_OUT_OF_RESOURCES; > + OptionalData = (UINT8 *)NvRamMap->BootOptionalData; > + OptionalDataSize = (UINT32)StrSize (NvRamMap->BootOptionalData); > } > > NewMenuEntry = BOpt_CreateMenuEntry (BM_LOAD_CONTEXT_SELECT); > if (NULL == NewMenuEntry) { > return EFI_OUT_OF_RESOURCES; > } > > + Status = EfiBootManagerInitializeLoadOption ( > + &LoadOption, > + Index, > + LoadOptionTypeBoot, > + LOAD_OPTION_ACTIVE, > + NvRamMap->BootDescriptionData, > + CallbackData->LoadContext->FilePathList, > + OptionalData, > + OptionalDataSize > + ); > + if (!EFI_ERROR (Status)){ > + Status = EfiBootManagerAddLoadOptionVariable (&LoadOption,(UINTN) -1 ); > + } > + > NewLoadContext = (BM_LOAD_CONTEXT *) > NewMenuEntry->VariableContext; > NewLoadContext->Deleted = FALSE; > - NewLoadContext->LoadOptionSize = BufferSize; > - Ptr = (UINT8 *) Buffer; > - NewLoadContext->LoadOption = Ptr; > - *((UINT32 *) Ptr) = LOAD_OPTION_ACTIVE; > - NewLoadContext->Attributes = *((UINT32 *) Ptr); > - NewLoadContext->IsActive = TRUE; > - NewLoadContext->ForceReconnect = (BOOLEAN) (NewLoadContext->Attributes & > LOAD_OPTION_FORCE_RECONNECT); > - > - Ptr += sizeof (UINT32); > - *((UINT16 *) Ptr) = (UINT16) GetDevicePathSize > (CallbackData->LoadContext->FilePathList); > - NewLoadContext->FilePathListLength = *((UINT16 *) Ptr); > - Ptr += sizeof (UINT16); > - > - CopyMem ( > - Ptr, > - NvRamMap->BootDescriptionData, > - StrSize (NvRamMap->BootDescriptionData) > - ); > + NewLoadContext->Attributes = LoadOption.Attributes; > + NewLoadContext->FilePathListLength = (UINT16) GetDevicePathSize > (LoadOption.FilePath); > > NewLoadContext->Description = AllocateZeroPool (StrSize > (NvRamMap->BootDescriptionData)); > ASSERT (NewLoadContext->Description != NULL); > > NewMenuEntry->DisplayString = NewLoadContext->Description; > + > CopyMem ( > NewLoadContext->Description, > - (VOID *) Ptr, > + LoadOption.Description, > StrSize (NvRamMap->BootDescriptionData) > ); > > - Ptr += StrSize (NvRamMap->BootDescriptionData); > - CopyMem ( > - Ptr, > - CallbackData->LoadContext->FilePathList, > - GetDevicePathSize (CallbackData->LoadContext->FilePathList) > - ); > - > NewLoadContext->FilePathList = AllocateZeroPool (GetDevicePathSize > (CallbackData->LoadContext->FilePathList)); > ASSERT (NewLoadContext->FilePathList != NULL); > - > CopyMem ( > NewLoadContext->FilePathList, > - (VOID *) Ptr, > + LoadOption.FilePath, > GetDevicePathSize (CallbackData->LoadContext->FilePathList) > ); > > NewMenuEntry->HelpString = UiDevicePathToStr > (NewLoadContext->FilePathList); > NewMenuEntry->OptionNumber = Index; > NewMenuEntry->DisplayStringToken = HiiSetString > (CallbackData->BmmHiiHandle, 0, NewMenuEntry->DisplayString, NULL); > NewMenuEntry->HelpStringToken = HiiSetString (CallbackData->BmmHiiHandle, > 0, NewMenuEntry->HelpString, NULL); > > if (OptionalDataExist) { > - Ptr += (UINT8) GetDevicePathSize > (CallbackData->LoadContext->FilePathList); > - > - CopyMem (Ptr, NvRamMap->BootOptionalData, StrSize > (NvRamMap->BootOptionalData)); > - } > - > - Status = gRT->SetVariable ( > - BootString, > - &gEfiGlobalVariableGuid, > - VAR_FLAG, > - BufferSize, > - Buffer > - ); > - ASSERT_EFI_ERROR (Status); > - > - GetEfiGlobalVariable2 (L"BootOrder", (VOID **) &BootOrderList, > &BootOrderListSize); > - NewBootOrderList = AllocateZeroPool (BootOrderListSize + sizeof (UINT16)); > - ASSERT (NewBootOrderList != NULL); > - if (BootOrderList != NULL){ > - CopyMem (NewBootOrderList, BootOrderList, BootOrderListSize); > - } > - NewBootOrderList[BootOrderListSize / sizeof (UINT16)] = Index; > - > - if (BootOrderList != NULL) { > - FreePool (BootOrderList); > + NewLoadContext->OptionalData = AllocateZeroPool > (LoadOption.OptionalDataSize); > + CopyMem ( > + NewLoadContext->OptionalData, > + LoadOption.OptionalData, > + LoadOption.OptionalDataSize > + ); > } > > - Status = gRT->SetVariable ( > - L"BootOrder", > - &gEfiGlobalVariableGuid, > - VAR_FLAG, > - BootOrderListSize + sizeof (UINT16), > - NewBootOrderList > - ); > - ASSERT_EFI_ERROR (Status); > - > - FreePool (NewBootOrderList); > - NewBootOrderList = NULL; > InsertTailList (&BootOptionMenu.Head, &NewMenuEntry->Link); > BootOptionMenu.MenuNumber++; > > + EfiBootManagerFreeLoadOption(&LoadOption); > + > return EFI_SUCCESS; > } > > /** > This function update the "BootNext" EFI Variable. If there is > -- > 1.9.5.msysgit.1
_______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
