Re: [edk2-devel] [PATCH v2 1/1] ShellPkg: Tidy for code readability

["Gao, Zhichao" <zhichao.gao@intel.com>]

The patch looks good to me. Reviewed-by: Zhichao Gao <zhichao.gao@intel.com>

Thanks,
Zhichao

> -----Original Message-----
> From: Tom Pilar <Tom.Pilar@arm.com>
> Sent: Wednesday, December 13, 2023 7:45 PM
> To: devel@edk2.groups.io; Daniel Nguyen <Daniel.Nguyen@arm.com>
> Cc: nd <nd@arm.com>; Gao, Zhichao <zhichao.gao@intel.com>; Ni, Ray
> <ray.ni@intel.com>
> Subject: Re: [edk2-devel] [PATCH v2 1/1] ShellPkg: Tidy for code readability
> 
> Hi Zhichao & Ray,
> 
> Can you have a quick review of this patch, no logic has changed, just a code
> tidy.
> 
> Cheers,
> Tom
> 
> > On 29 Nov 2023, at 18:28, Daniel Nguyen via groups.io
> <daniel.nguyen=arm.com@groups.io> wrote:
> >
> > Warning: EXTERNAL SENDER, use caution when opening links or
> attachments.
> >
> >
> > Use error handling instead of success handling.
> > Less indented logic is easier to read.
> >
> > Cc: Zhichao Gao <zhichao.gao@intel.com>
> > Cc: Ray Ni <ray.ni@intel.com>
> >
> > Signed-off-by: Daniel Nguyen <daniel.nguyen@arm.com>
> > ---
> > ShellPkg/Library/UefiShellLevel2CommandsLib/Reset.c | 41
> > +++++++++++---------
> > 1 file changed, 23 insertions(+), 18 deletions(-)
> >
> > diff --git a/ShellPkg/Library/UefiShellLevel2CommandsLib/Reset.c
> > b/ShellPkg/Library/UefiShellLevel2CommandsLib/Reset.c
> > index 57ba3c90f373..361c47e43059 100644
> > --- a/ShellPkg/Library/UefiShellLevel2CommandsLib/Reset.c
> > +++ b/ShellPkg/Library/UefiShellLevel2CommandsLib/Reset.c
> > @@ -79,30 +79,35 @@ ShellCommandRunReset (
> >                           &DataSize,
> >                           &OsIndications
> >                           );
> > -        if (!EFI_ERROR (Status)) {
> > -          if ((OsIndications & EFI_OS_INDICATIONS_BOOT_TO_FW_UI) != 0) {
> > -            DataSize = sizeof (OsIndications);
> > -            Status   = gRT->GetVariable (
> > -                              EFI_OS_INDICATIONS_VARIABLE_NAME,
> > -                              &gEfiGlobalVariableGuid,
> > -                              &Attr,
> > -                              &DataSize,
> > -                              &OsIndications
> > -                              );
> > -            if (!EFI_ERROR (Status)) {
> > -              OsIndications |= EFI_OS_INDICATIONS_BOOT_TO_FW_UI;
> > -            } else {
> > -              OsIndications = EFI_OS_INDICATIONS_BOOT_TO_FW_UI;
> > -            }
> >
> > -            Status = gRT->SetVariable (
> > +        if (EFI_ERROR (Status)) {
> > +          ShellStatus = SHELL_UNSUPPORTED;
> > +          goto Error;
> > +        }
> > +
> > +        if ((OsIndications & EFI_OS_INDICATIONS_BOOT_TO_FW_UI) != 0) {
> > +          DataSize = sizeof (OsIndications);
> > +          Status   = gRT->GetVariable (
> >                             EFI_OS_INDICATIONS_VARIABLE_NAME,
> >                             &gEfiGlobalVariableGuid,
> > -                            EFI_VARIABLE_NON_VOLATILE |
> EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS,
> > -                            sizeof (OsIndications),
> > +                            &Attr,
> > +                            &DataSize,
> >                             &OsIndications
> >                             );
> > +
> > +          if (EFI_ERROR (Status)) {
> > +            OsIndications = EFI_OS_INDICATIONS_BOOT_TO_FW_UI;
> > +          } else {
> > +            OsIndications |= EFI_OS_INDICATIONS_BOOT_TO_FW_UI;
> >           }
> > +
> > +          Status = gRT->SetVariable (
> > +                          EFI_OS_INDICATIONS_VARIABLE_NAME,
> > +                          &gEfiGlobalVariableGuid,
> > +                          EFI_VARIABLE_NON_VOLATILE |
> EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS,
> > +                          sizeof (OsIndications),
> > +                          &OsIndications
> > +                          );
> >         }
> >
> >         if (EFI_ERROR (Status)) {
> > --
> > 2.25.1
> >
> >
> >
> > -=-=-=-=-=-=
> > Groups.io Links: You receive all messages sent to this group.
> > View/Reply Online (#111856):
> > https://edk2.groups.io/g/devel/message/111856
> > Mute This Topic: https://groups.io/mt/102877864/7994090
> > Group Owner: devel+owner@edk2.groups.io
> > Unsubscribe: https://edk2.groups.io/g/devel/unsub [tom.pilar@arm.com]
> > -=-=-=-=-=-=
> >
> >



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112495): https://edk2.groups.io/g/devel/message/112495
Mute This Topic: https://groups.io/mt/102877864/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-