public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Dong, Eric" <eric.dong@intel.com>
To: "Bi, Dandan" <dandan.bi@intel.com>,
	Laszlo Ersek <lersek@redhat.com>,
	"edk2-devel@ml01.01.org" <edk2-devel@ml01.01.org>
Cc: "Gao, Liming" <liming.gao@intel.com>
Subject: Re: [PATCH v2] MdeModulePkg/BootMaintenanceUi: Enhance the codes logic
Date: Wed, 19 Oct 2016 01:36:13 +0000	[thread overview]
Message-ID: <ED077930C258884BBCB450DB737E662248681AAD@shsmsx102.ccr.corp.intel.com> (raw)
In-Reply-To: <3C0D5C461C9E904E8F62152F6274C0BB39639B97@shsmsx102.ccr.corp.intel.com>

Reviewed-by: Eric Dong <eric.dong@intel.com>

> -----Original Message-----
> From: Bi, Dandan
> Sent: Monday, October 17, 2016 5:08 PM
> To: Laszlo Ersek; edk2-devel@ml01.01.org
> Cc: Dong, Eric; Gao, Liming
> Subject: RE: [edk2] [PATCH v2] MdeModulePkg/BootMaintenanceUi: Enhance the codes logic
> 
> 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: ...
> 
> 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 <dandan.bi@intel.com>; edk2-devel@ml01.01.org
> Cc: Dong, Eric <eric.dong@intel.com>; Gao, Liming <liming.gao@intel.com>
> 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 <liming.gao@intel.com>
> > Cc: Eric Dong <eric.dong@intel.com>
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Dandan Bi <dandan.bi@intel.com>
> > ---
> >  .../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 <ConfigRequest> 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 <ConfigResp> 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;
> >



      parent reply	other threads:[~2016-10-19  1:36 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-10-14  6:43 [PATCH v2] MdeModulePkg/BootMaintenanceUi: Enhance the codes logic Dandan Bi
2016-10-14 12:34 ` Laszlo Ersek
2016-10-17  9:07   ` Bi, Dandan
2016-10-17  9:57     ` Laszlo Ersek
2016-10-19  1:36     ` Dong, Eric [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ED077930C258884BBCB450DB737E662248681AAD@shsmsx102.ccr.corp.intel.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox