Jaben, CommandInit() API initializes gUnicodeCollation pointer. But ShellCommandLibConstructor() also initializes the gUnicodeCollation pointer.
Can we just remove CommandInit() from this library? And update all callers? Thanks/Ray > -----Original Message----- > From: edk2-devel [mailto:[email protected]] On Behalf Of > Carsey, Jaben > Sent: Thursday, June 30, 2016 11:13 PM > To: Laszlo Ersek <[email protected]>; Qiu, Shumin <[email protected]> > Cc: Carsey, Jaben <[email protected]>; edk2-devel-01 <edk2- > [email protected]> > Subject: Re: [edk2] [PATCH 4/6] ShellPkg: don't call functions with side > effects in ASSERT_EFI_ERROR() > > Reviewed-by: Jaben Carsey <[email protected]> > > > -----Original Message----- > > From: Laszlo Ersek [mailto:[email protected]] > > Sent: Thursday, June 30, 2016 4:07 AM > > To: Carsey, Jaben <[email protected]>; Qiu, Shumin > > <[email protected]> > > Cc: edk2-devel-01 <[email protected]> > > Subject: Re: [edk2] [PATCH 4/6] ShellPkg: don't call functions with > > side effects in ASSERT_EFI_ERROR() > > Importance: High > > > > Jaben, Shumin, > > > > On 06/28/16 15:25, Laszlo Ersek wrote: > > > When ASSERT_EFI_ERROR() is compiled out, dependent on build flags, > > > only the status checking should be removed; the function calls should > stay. > > > > > > Cc: Jaben Carsey <[email protected]> > > > Cc: Shumin Qiu <[email protected]> > > > Contributed-under: TianoCore Contribution Agreement 1.0 > > > Signed-off-by: Laszlo Ersek <[email protected]> > > > --- > > > > > > Notes: > > > build tested > > > > > > ShellPkg/Library/UefiShellLevel1CommandsLib/If.c | 10 ++++++++-- > > > ShellPkg/Library/UefiShellLib/UefiShellLib.c | 5 ++++- > > > 2 files changed, 12 insertions(+), 3 deletions(-) > > > > Can I please get a maintainer review for this patch? > > > > Thanks > > Laszlo > > > > > diff --git a/ShellPkg/Library/UefiShellLevel1CommandsLib/If.c > > b/ShellPkg/Library/UefiShellLevel1CommandsLib/If.c > > > index 7abfd8944b92..dc96bffde7d3 100644 > > > --- a/ShellPkg/Library/UefiShellLevel1CommandsLib/If.c > > > +++ b/ShellPkg/Library/UefiShellLevel1CommandsLib/If.c > > > @@ -991,8 +991,11 @@ ShellCommandRunElse ( > > > IN EFI_SYSTEM_TABLE *SystemTable > > > ) > > > { > > > + EFI_STATUS Status; > > > SCRIPT_FILE *CurrentScriptFile; > > > - ASSERT_EFI_ERROR(CommandInit()); > > > + > > > + Status = CommandInit (); > > > + ASSERT_EFI_ERROR (Status); > > > > > > if (gEfiShellParametersProtocol->Argc > 1) { > > > ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN (STR_GEN_TOO_MANY), > > gShellLevel1HiiHandle, L"if"); > > > @@ -1066,8 +1069,11 @@ ShellCommandRunEndIf ( > > > IN EFI_SYSTEM_TABLE *SystemTable > > > ) > > > { > > > + EFI_STATUS Status; > > > SCRIPT_FILE *CurrentScriptFile; > > > - ASSERT_EFI_ERROR(CommandInit()); > > > + > > > + Status = CommandInit (); > > > + ASSERT_EFI_ERROR (Status); > > > > > > if (gEfiShellParametersProtocol->Argc > 1) { > > > ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN (STR_GEN_TOO_MANY), > > gShellLevel1HiiHandle, L"if"); > > > diff --git a/ShellPkg/Library/UefiShellLib/UefiShellLib.c > > b/ShellPkg/Library/UefiShellLib/UefiShellLib.c > > > index cf89a4ac87ed..35a1a7169c8b 100644 > > > --- a/ShellPkg/Library/UefiShellLib/UefiShellLib.c > > > +++ b/ShellPkg/Library/UefiShellLib/UefiShellLib.c > > > @@ -373,6 +373,8 @@ EFIAPI > > > ShellInitialize ( > > > ) > > > { > > > + EFI_STATUS Status; > > > + > > > // > > > // if auto initialize is not false then skip > > > // > > > @@ -383,7 +385,8 @@ ShellInitialize ( > > > // > > > // deinit the current stuff > > > // > > > - ASSERT_EFI_ERROR(ShellLibDestructor(gImageHandle, gST)); > > > + Status = ShellLibDestructor (gImageHandle, gST); > > > + ASSERT_EFI_ERROR (Status); > > > > > > // > > > // init the new stuff > > > > > _______________________________________________ > edk2-devel mailing list > [email protected] > https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
