The patch looks good to me. Reviewed-by: Zhichao Gao <zhichao....@intel.com>
Thanks,
Zhichao
> -----Original Message-----
> From: Tom Pilar <tom.pi...@arm.com>
> Sent: Wednesday, December 13, 2023 7:45 PM
> To: devel@edk2.groups.io; Daniel Nguyen <daniel.ngu...@arm.com>
> Cc: nd <n...@arm.com>; Gao, Zhichao <zhichao....@intel.com>; Ni, Ray
> <ray...@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....@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....@intel.com>
> > Cc: Ray Ni <ray...@intel.com>
> >
> > Signed-off-by: Daniel Nguyen <daniel.ngu...@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+ow...@edk2.groups.io
> > Unsubscribe: https://edk2.groups.io/g/devel/unsub [tom.pi...@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/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
