From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 2111B1A1E24 for ; Mon, 17 Oct 2016 02:57:41 -0700 (PDT) Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 8D130624B1; Mon, 17 Oct 2016 09:57:40 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-91.phx2.redhat.com [10.3.116.91]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u9H9vc5G028801; Mon, 17 Oct 2016 05:57:38 -0400 To: "Bi, Dandan" , "edk2-devel@ml01.01.org" References: <1476427423-72636-1-git-send-email-dandan.bi@intel.com> <3C0D5C461C9E904E8F62152F6274C0BB39639B97@shsmsx102.ccr.corp.intel.com> Cc: "Dong, Eric" , "Gao, Liming" From: Laszlo Ersek Message-ID: <745293d0-0ef8-6626-9634-95e6b3e13b54@redhat.com> Date: Mon, 17 Oct 2016 11:57:37 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <3C0D5C461C9E904E8F62152F6274C0BB39639B97@shsmsx102.ccr.corp.intel.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.23 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.39]); Mon, 17 Oct 2016 09:57:40 +0000 (UTC) Subject: Re: [PATCH v2] MdeModulePkg/BootMaintenanceUi: Enhance the codes logic X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 17 Oct 2016 09:57:41 -0000 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit On 10/17/16 11:07, Bi, Dandan wrote: > Hi Laszlo, > > Thank you very much for your comments! > I have split this patch into 5 independent patches with following subject : > [patch 1/5] MdeModulePkg/BMMUI: ... > [patch 2/5] MdeModulePkg/BMMUI: ... > [patch 3/5] MdeModulePkg/BMMUI: ... > [patch 4/5] MdeModulePkg/BMMUI: ... > [patch 5/5] MdeModulePkg/BMMUI: ... Thank you very much -- the new subjects look much-much better. Also, I think it's a nice trick to shorten "Boot Maintenance Manager UI" as "BMMUI", it's understandable and it saves quite a few characters in the subjects. Thanks! Laszlo > Hi Liming/Eric > Please review the new patches and ignore this one. Sorry for any inconvenience. > > > Regards, > Dandan > > -----Original Message----- > From: Laszlo Ersek [mailto:lersek@redhat.com] > Sent: Friday, October 14, 2016 8:34 PM > To: Bi, Dandan ; edk2-devel@ml01.01.org > Cc: Dong, Eric ; Gao, Liming > Subject: Re: [edk2] [PATCH v2] MdeModulePkg/BootMaintenanceUi: Enhance the codes logic > > On 10/14/16 08:43, Dandan Bi wrote: >> This patch is mainly to: >> 1. Enhance the error handling codes when set variable fail. >> 2. Enhance the logic to fix some incorrect UI behaviors. > > My apologies, but both the subject line and the commit message are mostly impenetrable. > > This patch should be split up into a series of two patches, minimally (according to the two goals above that it implements), and each change should be described correctly in both the subject line and in the commit message. > > If I got a bug report for OVMF that I managed to bisect back to this patch, I'd be *completely* helpless figuring out what it does. > > What kind of variables are set by the code? What happens now if setting those variables fails? What is the expected behavior instead that the > (first) patch implements? > > What are those incorrect UI behaviors? When do they happen? What does the second patch do to address those issues? > > Dear Developers, please *stop* writing subject lines like > > "Enhance the code in DNS driver" > "Enhance the codes logic" > > those subject lines are *completely* useless. You could replace all those subject lines, without any loss of information, with the following > one: > > "Do Work" > > Please spend time thinking about the granularity, the focus of your patches, as a *standalone activity* during development. Ask yourselves, "Is this patch small enough? Am I doing two or more independent things here? Is the subject line clear enough? If a person sees the code for the first time, will my commit message help them?" > > You don't write the commit message for yourselves only, you write it for other developers who might have absolutely no clue what's going on in your module. > > Thanks > Laszlo > >> V2: Update the Console/Terminal menu when the related question changed. >> >> Cc: Liming Gao >> Cc: Eric Dong >> Contributed-under: TianoCore Contribution Agreement 1.0 >> Signed-off-by: Dandan Bi >> --- >> .../BootMaintenanceManagerUiLib/BootMaintenance.c | 390 ++++++++++++++++----- >> .../BootMaintenanceManagerUiLib/UpdatePage.c | 42 ++- >> .../Library/BootMaintenanceManagerUiLib/Variable.c | 28 +- >> 3 files changed, 357 insertions(+), 103 deletions(-) >> >> diff --git >> a/MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenance.c >> b/MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenance.c >> index a190596..924eb49 100644 >> --- >> a/MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenance.c >> +++ b/MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenance >> +++ .c >> @@ -442,10 +442,197 @@ BmmExtractDevicePathFromHiiHandle ( >> return ConvertDevicePathToText(DevicePathFromHandle (DriverHandle), >> FALSE, FALSE); >> >> } >> >> /** >> + Converts the unicode character of the string from uppercase to lowercase. >> + This is a internal function. >> + >> + @param ConfigString String to be converted >> + >> +**/ >> +VOID >> +HiiToLower ( >> + IN EFI_STRING ConfigString >> + ) >> +{ >> + EFI_STRING String; >> + BOOLEAN Lower; >> + >> + ASSERT (ConfigString != NULL); >> + >> + // >> + // Convert all hex digits in range [A-F] in the configuration >> +header to [a-f] >> + // >> + for (String = ConfigString, Lower = FALSE; *String != L'\0'; String++) { >> + if (*String == L'=') { >> + Lower = TRUE; >> + } else if (*String == L'&') { >> + Lower = FALSE; >> + } else if (Lower && *String >= L'A' && *String <= L'F') { >> + *String = (CHAR16) (*String - L'A' + L'a'); >> + } >> + } >> +} >> + >> +/** >> + Update the progress string through the offset value. >> + >> + @param Offset The offset value >> + @param Configuration Point to the configuration string. >> + >> +**/ >> +EFI_STRING >> +UpdateProgress( >> + IN UINTN Offset, >> + IN EFI_STRING Configuration >> +) >> +{ >> + UINTN Length; >> + EFI_STRING StringPtr; >> + EFI_STRING ReturnString; >> + >> + StringPtr = NULL; >> + ReturnString = NULL; >> + >> + // >> + // &OFFSET=XXXX followed by a Null-terminator. >> + // Length = StrLen (L"&OFFSET=") + 4 + 1 // >> + Length = StrLen (L"&OFFSET=") + 4 + 1; >> + >> + StringPtr = AllocateZeroPool (Length * sizeof (CHAR16)); >> + >> + if (StringPtr == NULL) { >> + return NULL; >> + } >> + >> + UnicodeSPrint ( >> + StringPtr, >> + (8 + 4 + 1) * sizeof (CHAR16), >> + L"&OFFSET=%04x", >> + Offset >> + ); >> + >> + ReturnString = StrStr (Configuration, StringPtr); >> + >> + if (ReturnString == NULL) { >> + // >> + // If doesn't find the string in Configuration, convert the string to lower case then search again. >> + // >> + HiiToLower (StringPtr); >> + ReturnString = StrStr (Configuration, StringPtr); } >> + >> + FreePool (StringPtr); >> + >> + return ReturnString; >> +} >> + >> +/** >> + Update the terminal content in TerminalMenu. >> + >> + @param BmmData The BMM fake NV data. >> + >> +**/ >> +VOID >> +UpdateTerminalContent ( >> + IN BMM_FAKE_NV_DATA *BmmData >> + ) >> +{ >> + UINT16 Index; >> + BM_TERMINAL_CONTEXT *NewTerminalContext; >> + BM_MENU_ENTRY *NewMenuEntry; >> + >> + for (Index = 0; Index < TerminalMenu.MenuNumber; Index++) { >> + NewMenuEntry = BOpt_GetMenuEntry (&TerminalMenu, Index); >> + ASSERT (NewMenuEntry != NULL); >> + NewTerminalContext = (BM_TERMINAL_CONTEXT *) NewMenuEntry->VariableContext; >> + NewTerminalContext->BaudRateIndex = BmmData->COMBaudRate[Index]; >> + ASSERT (BmmData->COMBaudRate[Index] < (sizeof (BaudRateList) / sizeof (BaudRateList[0]))); >> + NewTerminalContext->BaudRate = BaudRateList[BmmData->COMBaudRate[Index]].Value; >> + NewTerminalContext->DataBitsIndex = BmmData->COMDataRate[Index]; >> + ASSERT (BmmData->COMDataRate[Index] < (sizeof (DataBitsList) / sizeof (DataBitsList[0]))); >> + NewTerminalContext->DataBits = (UINT8) DataBitsList[BmmData->COMDataRate[Index]].Value; >> + NewTerminalContext->StopBitsIndex = BmmData->COMStopBits[Index]; >> + ASSERT (BmmData->COMStopBits[Index] < (sizeof (StopBitsList) / sizeof (StopBitsList[0]))); >> + NewTerminalContext->StopBits = (UINT8) StopBitsList[BmmData->COMStopBits[Index]].Value; >> + NewTerminalContext->ParityIndex = BmmData->COMParity[Index]; >> + ASSERT (BmmData->COMParity[Index] < (sizeof (ParityList) / sizeof (ParityList[0]))); >> + NewTerminalContext->Parity = (UINT8) ParityList[BmmData->COMParity[Index]].Value; >> + NewTerminalContext->TerminalType = BmmData->COMTerminalType[Index]; >> + NewTerminalContext->FlowControl = BmmData->COMFlowControl[Index]; >> + ChangeTerminalDevicePath ( >> + NewTerminalContext->DevicePath, >> + FALSE >> + ); >> + } >> +} >> + >> +/** >> + Update the console content in ConsoleMenu. >> + >> + @param BmmData The BMM fake NV data. >> + >> +**/ >> +VOID >> +UpdateConsoleContent( >> + IN CHAR16 *ConsoleName, >> + IN BMM_FAKE_NV_DATA *BmmData >> + ) >> +{ >> + UINT16 Index; >> + BM_CONSOLE_CONTEXT *NewConsoleContext; >> + BM_TERMINAL_CONTEXT *NewTerminalContext; >> + BM_MENU_ENTRY *NewMenuEntry; >> + >> + if (StrCmp (ConsoleName, L"ConIn") == 0) { >> + for (Index = 0; Index < ConsoleInpMenu.MenuNumber; Index++){ >> + NewMenuEntry = BOpt_GetMenuEntry(&ConsoleInpMenu, Index); >> + NewConsoleContext = (BM_CONSOLE_CONTEXT *)NewMenuEntry->VariableContext; >> + ASSERT (Index < MAX_MENU_NUMBER); >> + NewConsoleContext->IsActive = BmmData->ConsoleInCheck[Index]; >> + } >> + for (Index = 0; Index < TerminalMenu.MenuNumber; Index++) { >> + NewMenuEntry = BOpt_GetMenuEntry (&TerminalMenu, Index); >> + NewTerminalContext = (BM_TERMINAL_CONTEXT *) NewMenuEntry->VariableContext; >> + ASSERT (Index + ConsoleInpMenu.MenuNumber < MAX_MENU_NUMBER); >> + NewTerminalContext->IsConIn = BmmData->ConsoleInCheck[Index + ConsoleInpMenu.MenuNumber]; >> + } >> + } >> + >> + if (StrCmp (ConsoleName, L"ConOut") == 0) { >> + for (Index = 0; Index < ConsoleOutMenu.MenuNumber; Index++){ >> + NewMenuEntry = BOpt_GetMenuEntry(&ConsoleOutMenu, Index); >> + NewConsoleContext = (BM_CONSOLE_CONTEXT *)NewMenuEntry->VariableContext; >> + ASSERT (Index < MAX_MENU_NUMBER); >> + NewConsoleContext->IsActive = BmmData->ConsoleOutCheck[Index]; >> + } >> + for (Index = 0; Index < TerminalMenu.MenuNumber; Index++) { >> + NewMenuEntry = BOpt_GetMenuEntry (&TerminalMenu, Index); >> + NewTerminalContext = (BM_TERMINAL_CONTEXT *) NewMenuEntry->VariableContext; >> + ASSERT (Index + ConsoleOutMenu.MenuNumber < MAX_MENU_NUMBER); >> + NewTerminalContext->IsConOut = BmmData->ConsoleOutCheck[Index + ConsoleOutMenu.MenuNumber]; >> + } >> + } >> + if (StrCmp (ConsoleName, L"ErrOut") == 0) { >> + for (Index = 0; Index < ConsoleErrMenu.MenuNumber; Index++){ >> + NewMenuEntry = BOpt_GetMenuEntry(&ConsoleErrMenu, Index); >> + NewConsoleContext = (BM_CONSOLE_CONTEXT *)NewMenuEntry->VariableContext; >> + ASSERT (Index < MAX_MENU_NUMBER); >> + NewConsoleContext->IsActive = BmmData->ConsoleErrCheck[Index]; >> + } >> + for (Index = 0; Index < TerminalMenu.MenuNumber; Index++) { >> + NewMenuEntry = BOpt_GetMenuEntry (&TerminalMenu, Index); >> + NewTerminalContext = (BM_TERMINAL_CONTEXT *) NewMenuEntry->VariableContext; >> + ASSERT (Index + ConsoleErrMenu.MenuNumber < MAX_MENU_NUMBER); >> + NewTerminalContext->IsStdErr = BmmData->ConsoleErrCheck[Index + ConsoleErrMenu.MenuNumber]; >> + } >> + } >> +} >> + >> +/** >> This function allows a caller to extract the current configuration for one >> or more named elements from the target driver. >> >> @param This Points to the EFI_HII_CONFIG_ACCESS_PROTOCOL. >> @param Request A null-terminated Unicode string in format. >> @@ -587,17 +774,16 @@ BootMaintRouteConfig ( >> EFI_STATUS Status; >> UINTN BufferSize; >> EFI_HII_CONFIG_ROUTING_PROTOCOL *ConfigRouting; >> BMM_FAKE_NV_DATA *NewBmmData; >> BMM_FAKE_NV_DATA *OldBmmData; >> - BM_CONSOLE_CONTEXT *NewConsoleContext; >> - BM_TERMINAL_CONTEXT *NewTerminalContext; >> BM_MENU_ENTRY *NewMenuEntry; >> BM_LOAD_CONTEXT *NewLoadContext; >> UINT16 Index; >> BOOLEAN TerminalAttChange; >> - BMM_CALLBACK_DATA *Private; >> + BMM_CALLBACK_DATA *Private; >> + UINTN Offset; >> >> if (Progress == NULL) { >> return EFI_INVALID_PARAMETER; >> } >> *Progress = Configuration; >> @@ -628,10 +814,11 @@ BootMaintRouteConfig ( >> // Get Buffer Storage data from EFI variable >> // >> BufferSize = sizeof (BMM_FAKE_NV_DATA); >> OldBmmData = &Private->BmmOldFakeNVData; >> NewBmmData = &Private->BmmFakeNvData; >> + Offset = 0; >> // >> // Convert to buffer data by helper function ConfigToBlock() >> // >> Status = ConfigRouting->ConfigToBlock ( >> ConfigRouting, @@ -649,10 +836,14 @@ >> BootMaintRouteConfig ( >> // >> // Check data which located in BMM main page and save the settings if need >> // >> if (CompareMem (&NewBmmData->BootNext, &OldBmmData->BootNext, sizeof (NewBmmData->BootNext)) != 0) { >> Status = Var_UpdateBootNext (Private); >> + if (EFI_ERROR (Status)) { >> + Offset = OFFSET_OF (BMM_FAKE_NV_DATA, BootNext); >> + goto Exit; >> + } >> } >> >> // >> // Check data which located in Boot Options Menu and save the settings if need >> // >> @@ -665,15 +856,23 @@ BootMaintRouteConfig ( >> NewLoadContext->Deleted = NewBmmData->BootOptionDel[Index]; >> NewBmmData->BootOptionDel[Index] = FALSE; >> NewBmmData->BootOptionDelMark[Index] = FALSE; >> } >> >> - Var_DelBootOption (); >> + Status = Var_DelBootOption (); >> + if (EFI_ERROR (Status)) { >> + Offset = OFFSET_OF (BMM_FAKE_NV_DATA, BootOptionDel); >> + goto Exit; >> + } >> } >> >> if (CompareMem (NewBmmData->BootOptionOrder, OldBmmData->BootOptionOrder, sizeof (NewBmmData->BootOptionOrder)) != 0) { >> Status = Var_UpdateBootOrder (Private); >> + if (EFI_ERROR (Status)) { >> + Offset = OFFSET_OF (BMM_FAKE_NV_DATA, BootOptionOrder); >> + goto Exit; >> + } >> } >> >> if (CompareMem (&NewBmmData->BootTimeOut, &OldBmmData->BootTimeOut, sizeof (NewBmmData->BootTimeOut)) != 0){ >> Status = gRT->SetVariable( >> L"Timeout", >> @@ -681,19 +880,12 @@ BootMaintRouteConfig ( >> EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_NON_VOLATILE, >> sizeof(UINT16), >> &(NewBmmData->BootTimeOut) >> ); >> if (EFI_ERROR (Status)) { >> - // >> - // If set variable fail, and don't have the appropriate error status for RouteConfig fuction to return, >> - // just return the EFI_NOT_FOUND. >> - // >> - if (Status == EFI_OUT_OF_RESOURCES) { >> - return Status; >> - } else { >> - return EFI_NOT_FOUND; >> - } >> + Offset = OFFSET_OF (BMM_FAKE_NV_DATA, BootTimeOut); >> + goto Exit; >> } >> Private->BmmOldFakeNVData.BootTimeOut = NewBmmData->BootTimeOut; >> } >> >> // >> @@ -707,19 +899,31 @@ BootMaintRouteConfig ( >> NewLoadContext = (BM_LOAD_CONTEXT *) NewMenuEntry->VariableContext; >> NewLoadContext->Deleted = NewBmmData->DriverOptionDel[Index]; >> NewBmmData->DriverOptionDel[Index] = FALSE; >> NewBmmData->DriverOptionDelMark[Index] = FALSE; >> } >> - Var_DelDriverOption (); >> + Status = Var_DelDriverOption (); >> + if (EFI_ERROR (Status)) { >> + Offset = OFFSET_OF (BMM_FAKE_NV_DATA, DriverOptionDel); >> + goto Exit; >> + } >> } >> >> if (CompareMem (NewBmmData->DriverOptionOrder, OldBmmData->DriverOptionOrder, sizeof (NewBmmData->DriverOptionOrder)) != 0) { >> Status = Var_UpdateDriverOrder (Private); >> + if (EFI_ERROR (Status)) { >> + Offset = OFFSET_OF (BMM_FAKE_NV_DATA, DriverOptionOrder); >> + goto Exit; >> + } >> } >> >> if (CompareMem (&NewBmmData->ConsoleOutMode, &OldBmmData->ConsoleOutMode, sizeof (NewBmmData->ConsoleOutMode)) != 0){ >> - Var_UpdateConMode(Private); >> + Status = Var_UpdateConMode(Private); >> + if (EFI_ERROR (Status)) { >> + Offset = OFFSET_OF (BMM_FAKE_NV_DATA, ConsoleOutMode); >> + goto Exit; >> + } >> } >> >> TerminalAttChange = FALSE; >> for (Index = 0; Index < TerminalMenu.MenuNumber; Index++) { >> >> @@ -733,95 +937,81 @@ BootMaintRouteConfig ( >> CompareMem (&NewBmmData->COMTerminalType[Index], &OldBmmData->COMTerminalType[Index], sizeof (NewBmmData->COMTerminalType[Index])) == 0 && >> CompareMem (&NewBmmData->COMFlowControl[Index], &OldBmmData->COMFlowControl[Index], sizeof (NewBmmData->COMFlowControl[Index])) == 0) { >> continue; >> } >> >> - NewMenuEntry = BOpt_GetMenuEntry (&TerminalMenu, Index); >> - ASSERT (NewMenuEntry != NULL); >> - NewTerminalContext = (BM_TERMINAL_CONTEXT *) NewMenuEntry->VariableContext; >> - NewTerminalContext->BaudRateIndex = NewBmmData->COMBaudRate[Index]; >> - ASSERT (NewBmmData->COMBaudRate[Index] < (sizeof (BaudRateList) / sizeof (BaudRateList[0]))); >> - NewTerminalContext->BaudRate = BaudRateList[NewBmmData->COMBaudRate[Index]].Value; >> - NewTerminalContext->DataBitsIndex = NewBmmData->COMDataRate[Index]; >> - ASSERT (NewBmmData->COMDataRate[Index] < (sizeof (DataBitsList) / sizeof (DataBitsList[0]))); >> - NewTerminalContext->DataBits = (UINT8) DataBitsList[NewBmmData->COMDataRate[Index]].Value; >> - NewTerminalContext->StopBitsIndex = NewBmmData->COMStopBits[Index]; >> - ASSERT (NewBmmData->COMStopBits[Index] < (sizeof (StopBitsList) / sizeof (StopBitsList[0]))); >> - NewTerminalContext->StopBits = (UINT8) StopBitsList[NewBmmData->COMStopBits[Index]].Value; >> - NewTerminalContext->ParityIndex = NewBmmData->COMParity[Index]; >> - ASSERT (NewBmmData->COMParity[Index] < (sizeof (ParityList) / sizeof (ParityList[0]))); >> - NewTerminalContext->Parity = (UINT8) ParityList[NewBmmData->COMParity[Index]].Value; >> - NewTerminalContext->TerminalType = NewBmmData->COMTerminalType[Index]; >> - NewTerminalContext->FlowControl = NewBmmData->COMFlowControl[Index]; >> - ChangeTerminalDevicePath ( >> - NewTerminalContext->DevicePath, >> - FALSE >> - ); >> TerminalAttChange = TRUE; >> } >> if (TerminalAttChange) { >> - Var_UpdateConsoleInpOption (); >> - Var_UpdateConsoleOutOption (); >> - Var_UpdateErrorOutOption (); >> + if (CompareMem (&NewBmmData->COMBaudRate[Index], &OldBmmData->COMBaudRate[Index], sizeof (NewBmmData->COMBaudRate[Index])) != 0) { >> + Offset = OFFSET_OF (BMM_FAKE_NV_DATA, COMBaudRate); >> + } else if (CompareMem (&NewBmmData->COMDataRate[Index], &OldBmmData->COMDataRate[Index], sizeof (NewBmmData->COMDataRate[Index])) != 0) { >> + Offset = OFFSET_OF (BMM_FAKE_NV_DATA, COMDataRate); >> + } else if (CompareMem (&NewBmmData->COMStopBits[Index], &OldBmmData->COMStopBits[Index], sizeof (NewBmmData->COMStopBits[Index])) != 0) { >> + Offset = OFFSET_OF (BMM_FAKE_NV_DATA, COMStopBits); >> + } else if (CompareMem (&NewBmmData->COMParity[Index], &OldBmmData->COMParity[Index], sizeof (NewBmmData->COMParity[Index])) != 0) { >> + Offset = OFFSET_OF (BMM_FAKE_NV_DATA, COMParity); >> + } else if (CompareMem (&NewBmmData->COMTerminalType[Index], &OldBmmData->COMTerminalType[Index], sizeof (NewBmmData->COMTerminalType[Index])) != 0) { >> + Offset = OFFSET_OF (BMM_FAKE_NV_DATA, COMTerminalType); >> + } else if (CompareMem (&NewBmmData->COMFlowControl[Index], &OldBmmData->COMFlowControl[Index], sizeof (NewBmmData->COMFlowControl[Index])) != 0) { >> + Offset = OFFSET_OF (BMM_FAKE_NV_DATA, COMFlowControl); >> + } >> + UpdateTerminalContent (NewBmmData); >> + Status = Var_UpdateConsoleInpOption (); >> + if (EFI_ERROR (Status)) { >> + goto Exit; >> + } >> + Status = Var_UpdateConsoleOutOption (); >> + if (EFI_ERROR (Status)) { >> + goto Exit; >> + } >> + Status = Var_UpdateErrorOutOption (); >> + if (EFI_ERROR (Status)) { >> + goto Exit; >> + } >> } >> // >> // Check data which located in Console Options Menu and save the settings if need >> // >> if (CompareMem (NewBmmData->ConsoleInCheck, OldBmmData->ConsoleInCheck, sizeof (NewBmmData->ConsoleInCheck)) != 0){ >> - for (Index = 0; Index < ConsoleInpMenu.MenuNumber; Index++){ >> - NewMenuEntry = BOpt_GetMenuEntry(&ConsoleInpMenu, Index); >> - NewConsoleContext = (BM_CONSOLE_CONTEXT *)NewMenuEntry->VariableContext; >> - ASSERT (Index < MAX_MENU_NUMBER); >> - NewConsoleContext->IsActive = NewBmmData->ConsoleInCheck[Index]; >> - } >> - for (Index = 0; Index < TerminalMenu.MenuNumber; Index++) { >> - NewMenuEntry = BOpt_GetMenuEntry (&TerminalMenu, Index); >> - NewTerminalContext = (BM_TERMINAL_CONTEXT *) NewMenuEntry->VariableContext; >> - ASSERT (Index + ConsoleInpMenu.MenuNumber < MAX_MENU_NUMBER); >> - NewTerminalContext->IsConIn = NewBmmData->ConsoleInCheck[Index + ConsoleInpMenu.MenuNumber]; >> + UpdateConsoleContent (L"ConIn", NewBmmData); >> + Status = Var_UpdateConsoleInpOption(); >> + if (EFI_ERROR (Status)) { >> + Offset = OFFSET_OF (BMM_FAKE_NV_DATA, ConsoleInCheck); >> + goto Exit; >> } >> - Var_UpdateConsoleInpOption(); >> } >> >> if (CompareMem (NewBmmData->ConsoleOutCheck, OldBmmData->ConsoleOutCheck, sizeof (NewBmmData->ConsoleOutCheck)) != 0){ >> - for (Index = 0; Index < ConsoleOutMenu.MenuNumber; Index++){ >> - NewMenuEntry = BOpt_GetMenuEntry(&ConsoleOutMenu, Index); >> - NewConsoleContext = (BM_CONSOLE_CONTEXT *)NewMenuEntry->VariableContext; >> - ASSERT (Index < MAX_MENU_NUMBER); >> - NewConsoleContext->IsActive = NewBmmData->ConsoleOutCheck[Index]; >> - } >> - for (Index = 0; Index < TerminalMenu.MenuNumber; Index++) { >> - NewMenuEntry = BOpt_GetMenuEntry (&TerminalMenu, Index); >> - NewTerminalContext = (BM_TERMINAL_CONTEXT *) NewMenuEntry->VariableContext; >> - ASSERT (Index + ConsoleOutMenu.MenuNumber < MAX_MENU_NUMBER); >> - NewTerminalContext->IsConOut = NewBmmData->ConsoleOutCheck[Index + ConsoleOutMenu.MenuNumber]; >> + UpdateConsoleContent (L"ConOut", NewBmmData); >> + Status = Var_UpdateConsoleOutOption(); >> + if (EFI_ERROR (Status)) { >> + Offset = OFFSET_OF (BMM_FAKE_NV_DATA, ConsoleOutCheck); >> + goto Exit; >> } >> - Var_UpdateConsoleOutOption(); >> } >> >> if (CompareMem (NewBmmData->ConsoleErrCheck, OldBmmData->ConsoleErrCheck, sizeof (NewBmmData->ConsoleErrCheck)) != 0){ >> - for (Index = 0; Index < ConsoleErrMenu.MenuNumber; Index++){ >> - NewMenuEntry = BOpt_GetMenuEntry(&ConsoleErrMenu, Index); >> - NewConsoleContext = (BM_CONSOLE_CONTEXT *)NewMenuEntry->VariableContext; >> - ASSERT (Index < MAX_MENU_NUMBER); >> - NewConsoleContext->IsActive = NewBmmData->ConsoleErrCheck[Index]; >> - } >> - for (Index = 0; Index < TerminalMenu.MenuNumber; Index++) { >> - NewMenuEntry = BOpt_GetMenuEntry (&TerminalMenu, Index); >> - NewTerminalContext = (BM_TERMINAL_CONTEXT *) NewMenuEntry->VariableContext; >> - ASSERT (Index + ConsoleErrMenu.MenuNumber < MAX_MENU_NUMBER); >> - NewTerminalContext->IsStdErr = NewBmmData->ConsoleErrCheck[Index + ConsoleErrMenu.MenuNumber]; >> + UpdateConsoleContent (L"ErrOut", NewBmmData); >> + Status = Var_UpdateErrorOutOption(); >> + if (EFI_ERROR (Status)) { >> + Offset = OFFSET_OF (BMM_FAKE_NV_DATA, ConsoleErrCheck); >> + goto Exit; >> } >> - Var_UpdateErrorOutOption(); >> } >> >> if (CompareMem (NewBmmData->BootDescriptionData, OldBmmData->BootDescriptionData, sizeof (NewBmmData->BootDescriptionData)) != 0 || >> CompareMem (NewBmmData->BootOptionalData, OldBmmData->BootOptionalData, sizeof (NewBmmData->BootOptionalData)) != 0) { >> Status = Var_UpdateBootOption (Private); >> NewBmmData->BootOptionChanged = FALSE; >> if (EFI_ERROR (Status)) { >> - return Status; >> + if (CompareMem (NewBmmData->BootDescriptionData, OldBmmData->BootDescriptionData, sizeof (NewBmmData->BootDescriptionData)) != 0) { >> + Offset = OFFSET_OF (BMM_FAKE_NV_DATA, BootDescriptionData); >> + } else { >> + Offset = OFFSET_OF (BMM_FAKE_NV_DATA, BootOptionalData); >> + } >> + goto Exit; >> } >> BOpt_GetBootOptions (Private); >> } >> >> if (CompareMem (NewBmmData->DriverDescriptionData, >> OldBmmData->DriverDescriptionData, sizeof (NewBmmData->DriverDescriptionData)) != 0 || @@ -834,11 +1024,16 @@ BootMaintRouteConfig ( >> NewBmmData->ForceReconnect >> ); >> NewBmmData->DriverOptionChanged = FALSE; >> NewBmmData->ForceReconnect = TRUE; >> if (EFI_ERROR (Status)) { >> - return Status; >> + if (CompareMem (NewBmmData->DriverDescriptionData, OldBmmData->DriverDescriptionData, sizeof (NewBmmData->DriverDescriptionData)) != 0) { >> + Offset = OFFSET_OF (BMM_FAKE_NV_DATA, DriverDescriptionData); >> + } else { >> + Offset = OFFSET_OF (BMM_FAKE_NV_DATA, DriverOptionalData); >> + } >> + goto Exit; >> } >> >> BOpt_GetDriverOptions (Private); >> } >> >> @@ -846,10 +1041,21 @@ BootMaintRouteConfig ( >> // After user do the save action, need to update OldBmmData. >> // >> CopyMem (OldBmmData, NewBmmData, sizeof (BMM_FAKE_NV_DATA)); >> >> return EFI_SUCCESS; >> + >> +Exit: >> + // >> + // Fail to save the data, update the progress string. >> + // >> + *Progress = UpdateProgress (Offset, Configuration); >> + if (Status == EFI_OUT_OF_RESOURCES) { >> + return Status; >> + } else { >> + return EFI_NOT_FOUND; >> + } >> } >> >> /** >> This function processes the results of changes in configuration. >> >> @@ -880,10 +1086,11 @@ BootMaintCallback ( >> ) >> { >> BMM_CALLBACK_DATA *Private; >> BM_MENU_ENTRY *NewMenuEntry; >> BMM_FAKE_NV_DATA *CurrentFakeNVMap; >> + BMM_FAKE_NV_DATA *OldFakeNVMap; >> UINTN Index; >> EFI_DEVICE_PATH_PROTOCOL * File; >> >> if (Action != EFI_BROWSER_ACTION_CHANGING && Action != EFI_BROWSER_ACTION_CHANGED && Action != EFI_BROWSER_ACTION_FORM_OPEN) { >> // >> @@ -914,10 +1121,11 @@ BootMaintCallback ( >> } >> // >> // Retrive uncommitted data from Form Browser >> // >> CurrentFakeNVMap = &Private->BmmFakeNvData; >> + OldFakeNVMap = &Private->BmmOldFakeNVData; >> HiiGetBrowserData (&mBootMaintGuid, mBootMaintStorageName, sizeof >> (BMM_FAKE_NV_DATA), (UINT8 *) CurrentFakeNVMap); >> >> if (Action == EFI_BROWSER_ACTION_CHANGING) { >> if (Value == NULL) { >> return EFI_INVALID_PARAMETER; >> @@ -1016,21 +1224,25 @@ BootMaintCallback ( >> *ActionRequest = EFI_BROWSER_ACTION_REQUEST_FORM_SUBMIT_EXIT; >> } else if (QuestionId == KEY_VALUE_NO_SAVE_AND_EXIT_DRIVER) { >> // >> // Discard changes and exit formset >> // >> - CurrentFakeNVMap->DriverOptionalData[0] = 0x0000; >> - CurrentFakeNVMap->DriverDescriptionData[0] = 0x0000; >> + ZeroMem (CurrentFakeNVMap->DriverOptionalData, sizeof (CurrentFakeNVMap->DriverOptionalData)); >> + ZeroMem (CurrentFakeNVMap->BootDescriptionData, sizeof (CurrentFakeNVMap->BootDescriptionData)); >> + ZeroMem (OldFakeNVMap->DriverOptionalData, sizeof (OldFakeNVMap->DriverOptionalData)); >> + ZeroMem (OldFakeNVMap->DriverDescriptionData, sizeof >> + (OldFakeNVMap->DriverDescriptionData)); >> CurrentFakeNVMap->DriverOptionChanged = FALSE; >> CurrentFakeNVMap->ForceReconnect = TRUE; >> *ActionRequest = EFI_BROWSER_ACTION_REQUEST_FORM_DISCARD_EXIT; >> } else if (QuestionId == KEY_VALUE_NO_SAVE_AND_EXIT_BOOT) { >> // >> // Discard changes and exit formset >> // >> - CurrentFakeNVMap->BootOptionalData[0] = 0x0000; >> - CurrentFakeNVMap->BootDescriptionData[0] = 0x0000; >> + ZeroMem (CurrentFakeNVMap->BootOptionalData, sizeof (CurrentFakeNVMap->BootOptionalData)); >> + ZeroMem (CurrentFakeNVMap->BootDescriptionData, sizeof (CurrentFakeNVMap->BootDescriptionData)); >> + ZeroMem (OldFakeNVMap->BootOptionalData, sizeof (OldFakeNVMap->BootOptionalData)); >> + ZeroMem (OldFakeNVMap->BootDescriptionData, sizeof >> + (OldFakeNVMap->BootDescriptionData)); >> CurrentFakeNVMap->BootOptionChanged = FALSE; >> *ActionRequest = EFI_BROWSER_ACTION_REQUEST_FORM_DISCARD_EXIT; >> } else if (QuestionId == KEY_VALUE_BOOT_DESCRIPTION || QuestionId == KEY_VALUE_BOOT_OPTION) { >> CurrentFakeNVMap->BootOptionChanged = TRUE; >> } else if (QuestionId == KEY_VALUE_DRIVER_DESCRIPTION || >> QuestionId == KEY_VALUE_DRIVER_OPTION) { @@ -1074,10 +1286,26 @@ >> BootMaintCallback ( >> >> default: >> break; >> } >> } >> + // >> + // Update the content in Terminal menu and Console menu here. >> + // >> + if ((QuestionId >= CON_IN_DEVICE_QUESTION_ID) && (QuestionId < CON_IN_DEVICE_QUESTION_ID + MAX_MENU_NUMBER)) { >> + UpdateConsoleContent (L"ConIn",CurrentFakeNVMap); >> + UpdateTerminalContent(CurrentFakeNVMap); >> + } else if ((QuestionId >= CON_OUT_DEVICE_QUESTION_ID) && (QuestionId < CON_OUT_DEVICE_QUESTION_ID + MAX_MENU_NUMBER)) { >> + UpdateConsoleContent (L"ConOut", CurrentFakeNVMap); >> + UpdateTerminalContent(CurrentFakeNVMap); >> + } else if ((QuestionId >= CON_ERR_DEVICE_QUESTION_ID) && (QuestionId < CON_ERR_DEVICE_QUESTION_ID + MAX_MENU_NUMBER)) { >> + UpdateConsoleContent (L"ConErr", CurrentFakeNVMap); >> + UpdateTerminalContent(CurrentFakeNVMap); >> + } >> } >> >> // >> // Pass changed uncommitted data back to Form Browser >> // >> diff --git >> a/MdeModulePkg/Library/BootMaintenanceManagerUiLib/UpdatePage.c >> b/MdeModulePkg/Library/BootMaintenanceManagerUiLib/UpdatePage.c >> index 9e79826..398c346 100644 >> --- a/MdeModulePkg/Library/BootMaintenanceManagerUiLib/UpdatePage.c >> +++ b/MdeModulePkg/Library/BootMaintenanceManagerUiLib/UpdatePage.c >> @@ -260,10 +260,11 @@ UpdateBootDelPage ( >> // CallbackData->BmmFakeNvData.BootOptionDelMark[Index] = FALSE means BDS knows the selected boot option has >> // deleted, browser maintains old useless info. So clear this info here, and later update this info to browser >> // through HiiSetBrowserData function. >> // >> CallbackData->BmmFakeNvData.BootOptionDel[Index] = FALSE; >> + CallbackData->BmmOldFakeNVData.BootOptionDel[Index] = FALSE; >> } >> >> HiiCreateCheckBoxOpCode ( >> mStartOpCodeHandle, >> (EFI_QUESTION_ID) (BOOT_OPTION_DEL_QUESTION_ID + Index), @@ >> -346,10 +347,11 @@ UpdateDrvDelPage ( >> // CallbackData->BmmFakeNvData.BootOptionDelMark[Index] = FALSE means BDS knows the selected boot option has >> // deleted, browser maintains old useless info. So clear this info here, and later update this info to browser >> // through HiiSetBrowserData function. >> // >> CallbackData->BmmFakeNvData.DriverOptionDel[Index] = FALSE; >> + CallbackData->BmmOldFakeNVData.DriverOptionDel[Index] = FALSE; >> } >> HiiCreateCheckBoxOpCode ( >> mStartOpCodeHandle, >> (EFI_QUESTION_ID) (DRIVER_OPTION_DEL_QUESTION_ID + Index), >> VARSTORE_ID_BOOT_MAINT, >> @@ -457,46 +459,36 @@ UpdateConsolePage ( >> BM_TERMINAL_CONTEXT *NewTerminalContext; >> UINT16 Index; >> UINT16 Index2; >> UINT8 CheckFlags; >> UINT8 *ConsoleCheck; >> - UINT8 *OldConsoleCheck; >> - UINTN ConsoleCheckSize; >> EFI_QUESTION_ID QuestionIdBase; >> UINT16 VariableOffsetBase; >> >> CallbackData->BmmAskSaveOrNot = TRUE; >> >> UpdatePageStart (CallbackData); >> >> ConsoleCheck = NULL; >> - OldConsoleCheck = NULL; >> QuestionIdBase = 0; >> VariableOffsetBase = 0; >> - ConsoleCheckSize = 0; >> >> switch (UpdatePageId) { >> case FORM_CON_IN_ID: >> ConsoleCheck = &CallbackData->BmmFakeNvData.ConsoleInCheck[0]; >> - OldConsoleCheck = &CallbackData->BmmOldFakeNVData.ConsoleInCheck[0]; >> - ConsoleCheckSize = sizeof (CallbackData->BmmFakeNvData.ConsoleInCheck); >> QuestionIdBase = CON_IN_DEVICE_QUESTION_ID; >> VariableOffsetBase = CON_IN_DEVICE_VAR_OFFSET; >> break; >> >> case FORM_CON_OUT_ID: >> ConsoleCheck = &CallbackData->BmmFakeNvData.ConsoleOutCheck[0]; >> - OldConsoleCheck = &CallbackData->BmmOldFakeNVData.ConsoleOutCheck[0]; >> - ConsoleCheckSize = sizeof (CallbackData->BmmFakeNvData.ConsoleOutCheck); >> QuestionIdBase = CON_OUT_DEVICE_QUESTION_ID; >> VariableOffsetBase = CON_OUT_DEVICE_VAR_OFFSET; >> break; >> >> case FORM_CON_ERR_ID: >> ConsoleCheck = &CallbackData->BmmFakeNvData.ConsoleErrCheck[0]; >> - OldConsoleCheck = &CallbackData->BmmOldFakeNVData.ConsoleErrCheck[0]; >> - ConsoleCheckSize = sizeof (CallbackData->BmmFakeNvData.ConsoleErrCheck); >> QuestionIdBase = CON_ERR_DEVICE_QUESTION_ID; >> VariableOffsetBase = CON_ERR_DEVICE_VAR_OFFSET; >> break; >> } >> ASSERT (ConsoleCheck != NULL); >> @@ -517,11 +509,11 @@ UpdateConsolePage ( >> (EFI_QUESTION_ID) (QuestionIdBase + Index), >> VARSTORE_ID_BOOT_MAINT, >> (UINT16) (VariableOffsetBase + Index), >> NewMenuEntry->DisplayStringToken, >> NewMenuEntry->HelpStringToken, >> - 0, >> + EFI_IFR_FLAG_CALLBACK, >> CheckFlags, >> NULL >> ); >> } >> >> @@ -546,20 +538,18 @@ UpdateConsolePage ( >> (EFI_QUESTION_ID) (QuestionIdBase + Index), >> VARSTORE_ID_BOOT_MAINT, >> (UINT16) (VariableOffsetBase + Index), >> NewMenuEntry->DisplayStringToken, >> NewMenuEntry->HelpStringToken, >> - 0, >> + EFI_IFR_FLAG_CALLBACK, >> CheckFlags, >> NULL >> ); >> >> Index++; >> } >> >> - CopyMem (OldConsoleCheck, ConsoleCheck, ConsoleCheckSize); >> - >> UpdatePageEnd (CallbackData); >> } >> >> /** >> Update the page's NV Map if user has changed the order @@ -593,18 >> +583,34 @@ UpdateOrderPage ( >> QuestionId = 0; >> VarOffset = 0; >> switch (UpdatePageId) { >> >> case FORM_BOOT_CHG_ID: >> - GetBootOrder (CallbackData); >> + // >> + // If the BootOptionOrder in the BmmFakeNvData are same with the date in the BmmOldFakeNVData, >> + // means all Boot Options has been save in BootOptionMenu, we can get the date from the menu. >> + // else means browser maintains some uncommitted date which are not saved in BootOptionMenu, >> + // so we should not get the data from BootOptionMenu to show it. >> + // >> + if (CompareMem (CallbackData->BmmFakeNvData.BootOptionOrder, CallbackData->BmmOldFakeNVData.BootOptionOrder, sizeof (CallbackData->BmmFakeNvData.BootOptionOrder)) == 0) { >> + GetBootOrder (CallbackData); >> + } >> OptionOrder = CallbackData->BmmFakeNvData.BootOptionOrder; >> QuestionId = BOOT_OPTION_ORDER_QUESTION_ID; >> VarOffset = BOOT_OPTION_ORDER_VAR_OFFSET; >> break; >> >> case FORM_DRV_CHG_ID: >> - GetDriverOrder (CallbackData); >> + // >> + // If the DriverOptionOrder in the BmmFakeNvData are same with the date in the BmmOldFakeNVData, >> + // means all Driver Options has been save in DriverOptionMenu, we can get the DriverOptionOrder from the menu. >> + // else means browser maintains some uncommitted date which are not saved in DriverOptionMenu, >> + // so we should not get the data from DriverOptionMenu to show it. >> + // >> + if (CompareMem (CallbackData->BmmFakeNvData.DriverOptionOrder, CallbackData->BmmOldFakeNVData.DriverOptionOrder, sizeof (CallbackData->BmmFakeNvData.DriverOptionOrder)) == 0) { >> + GetDriverOrder (CallbackData); >> + } >> OptionOrder = CallbackData->BmmFakeNvData.DriverOptionOrder; >> QuestionId = DRIVER_OPTION_ORDER_QUESTION_ID; >> VarOffset = DRIVER_OPTION_ORDER_VAR_OFFSET; >> break; >> } >> @@ -1035,15 +1041,19 @@ UpdateOptionPage( >> >> if(FormId == FORM_BOOT_ADD_ID){ >> if (!CallbackData->BmmFakeNvData.BootOptionChanged) { >> ZeroMem (CallbackData->BmmFakeNvData.BootOptionalData, sizeof (CallbackData->BmmFakeNvData.BootOptionalData)); >> ZeroMem (CallbackData->BmmFakeNvData.BootDescriptionData, >> sizeof (CallbackData->BmmFakeNvData.BootDescriptionData)); >> + ZeroMem (CallbackData->BmmOldFakeNVData.BootOptionalData, sizeof (CallbackData->BmmOldFakeNVData.BootOptionalData)); >> + ZeroMem (CallbackData->BmmOldFakeNVData.BootDescriptionData, >> + sizeof (CallbackData->BmmOldFakeNVData.BootDescriptionData)); >> } >> } else if (FormId == FORM_DRV_ADD_FILE_ID){ >> if (!CallbackData->BmmFakeNvData.DriverOptionChanged) { >> ZeroMem (CallbackData->BmmFakeNvData.DriverOptionalData, sizeof (CallbackData->BmmFakeNvData.DriverOptionalData)); >> ZeroMem (CallbackData->BmmFakeNvData.DriverDescriptionData, >> sizeof (CallbackData->BmmFakeNvData.DriverDescriptionData)); >> + ZeroMem (CallbackData->BmmOldFakeNVData.DriverOptionalData, sizeof (CallbackData->BmmOldFakeNVData.DriverOptionalData)); >> + ZeroMem (CallbackData->BmmOldFakeNVData.DriverDescriptionData, >> + sizeof (CallbackData->BmmOldFakeNVData.DriverDescriptionData)); >> } >> } >> >> RefreshUpdateData(); >> mStartLabel->Number = FormId; >> diff --git >> a/MdeModulePkg/Library/BootMaintenanceManagerUiLib/Variable.c >> b/MdeModulePkg/Library/BootMaintenanceManagerUiLib/Variable.c >> index b65d6a5..a2ae2a7 100644 >> --- a/MdeModulePkg/Library/BootMaintenanceManagerUiLib/Variable.c >> +++ b/MdeModulePkg/Library/BootMaintenanceManagerUiLib/Variable.c >> @@ -463,11 +463,13 @@ Var_UpdateErrorOutOption ( >> @param HiiHandle The HII handle associated with the BMM formset. >> @param DescriptionData The description of this driver option. >> @param OptionalData The optional load option. >> @param ForceReconnect If to force reconnect. >> >> - @retval EFI_OUT_OF_RESOURCES If not enought memory to complete the operation. >> + @retval other Contain some errors when excuting this function.See function >> + EfiBootManagerInitializeLoadOption/EfiBootManagerAddLoadOptionVariabl >> + for detail return information. >> @retval EFI_SUCCESS If function completes successfully. >> >> **/ >> EFI_STATUS >> Var_UpdateDriverOption ( >> @@ -523,12 +525,18 @@ Var_UpdateDriverOption ( >> DescriptionData, >> CallbackData->LoadContext->FilePathList, >> OptionalDesData, >> OptionalDataSize >> ); >> - if (!EFI_ERROR (Status)){ >> - Status = EfiBootManagerAddLoadOptionVariable (&LoadOption,(UINTN) -1 ); >> + if (EFI_ERROR (Status)){ >> + return Status; >> + } >> + >> + Status = EfiBootManagerAddLoadOptionVariable (&LoadOption,(UINTN) >> + -1 ); if (EFI_ERROR (Status)) { >> + EfiBootManagerFreeLoadOption(&LoadOption); >> + return Status; >> } >> >> NewLoadContext = (BM_LOAD_CONTEXT *) NewMenuEntry->VariableContext; >> NewLoadContext->Deleted = FALSE; >> NewLoadContext->Attributes = LoadOption.Attributes; @@ -580,11 >> +588,13 @@ Var_UpdateDriverOption ( >> the "BootOrder" list. It also append this Boot Opotion to the end >> of BootOptionMenu. >> >> @param CallbackData The BMM context data. >> >> - @retval EFI_OUT_OF_RESOURCES If not enought memory to complete the operation. >> + @retval other Contain some errors when excuting this function. See function >> + EfiBootManagerInitializeLoadOption/EfiBootManagerAddLoadOptionVariabl >> + for detail return information. >> @retval EFI_SUCCESS If function completes successfully. >> >> **/ >> EFI_STATUS >> Var_UpdateBootOption ( >> @@ -633,12 +643,18 @@ Var_UpdateBootOption ( >> NvRamMap->BootDescriptionData, >> CallbackData->LoadContext->FilePathList, >> OptionalData, >> OptionalDataSize >> ); >> - if (!EFI_ERROR (Status)){ >> - Status = EfiBootManagerAddLoadOptionVariable (&LoadOption,(UINTN) -1 ); >> + if (EFI_ERROR (Status)){ >> + return Status; >> + } >> + >> + Status = EfiBootManagerAddLoadOptionVariable (&LoadOption,(UINTN) >> + -1 ); if (EFI_ERROR (Status)) { >> + EfiBootManagerFreeLoadOption(&LoadOption); >> + return Status; >> } >> >> NewLoadContext = (BM_LOAD_CONTEXT *) NewMenuEntry->VariableContext; >> NewLoadContext->Deleted = FALSE; >> NewLoadContext->Attributes = LoadOption.Attributes; >> >