From e8036a9fbbe4bad60e562c6641ca121280244bad Mon Sep 17 00:00:00 2001 From: Pierre Gondois Date: Tue, 7 Apr 2026 18:16:56 +0200 Subject: [PATCH] ShellPkg/UefiShellLevel1: Return if ShellCommandLineParse() failed This patch aims to help breaking down the long function present in the ShellPkg and reduce complexity/nested code and conditions. Return directly if ShellCommandLineParse() returned an error Status. In such case, the "Package" that should be allocated by ShellCommandLineParse() is already freed in: ShellCommandLineParse() \-ShellCommandLineParseEx() \-InternalCommandLineParse() so there is no need to free it with ShellCommandLineFreeVarList(). No functional change should be induced by this patch. Signed-off-by: Pierre Gondois --- .../Library/UefiShellLevel1CommandsLib/Exit.c | 40 ++++++----- .../Library/UefiShellLevel1CommandsLib/Goto.c | 72 ++++++++++--------- .../UefiShellLevel1CommandsLib/Stall.c | 36 +++++----- 3 files changed, 77 insertions(+), 71 deletions(-) diff --git a/ShellPkg/Library/UefiShellLevel1CommandsLib/Exit.c b/ShellPkg/Library/UefiShellLevel1CommandsLib/Exit.c index 26877524a8..c144b41435 100644 --- a/ShellPkg/Library/UefiShellLevel1CommandsLib/Exit.c +++ b/ShellPkg/Library/UefiShellLevel1CommandsLib/Exit.c @@ -57,34 +57,36 @@ ShellCommandRunExit ( } else { ASSERT (FALSE); } - } else { - // - // return the specified error code - // - Return = ShellCommandLineGetRawValue (Package, 1); - if (Return != NULL) { - Status = ShellConvertStringToUint64 (Return, &RetVal, FALSE, FALSE); - if (EFI_ERROR (Status)) { - ShellPrintHiiDefaultEx (STRING_TOKEN (STR_GEN_PARAM_INV), gShellLevel1HiiHandle, L"exit", Return); - ShellStatus = SHELL_INVALID_PARAMETER; - } else { - // - // If we are in a batch file and /b then pass TRUE otherwise false... - // - ShellCommandRegisterExit ((BOOLEAN)(gEfiShellProtocol->BatchIsActive () && ShellCommandLineGetFlag (Package, L"/b")), RetVal); - ShellStatus = SHELL_SUCCESS; - } + return ShellStatus; + } + + // + // return the specified error code + // + Return = ShellCommandLineGetRawValue (Package, 1); + if (Return != NULL) { + Status = ShellConvertStringToUint64 (Return, &RetVal, FALSE, FALSE); + if (EFI_ERROR (Status)) { + ShellPrintHiiDefaultEx (STRING_TOKEN (STR_GEN_PARAM_INV), gShellLevel1HiiHandle, L"exit", Return); + ShellStatus = SHELL_INVALID_PARAMETER; } else { + // // If we are in a batch file and /b then pass TRUE otherwise false... // - ShellCommandRegisterExit ((BOOLEAN)(gEfiShellProtocol->BatchIsActive () && ShellCommandLineGetFlag (Package, L"/b")), 0); + ShellCommandRegisterExit ((BOOLEAN)(gEfiShellProtocol->BatchIsActive () && ShellCommandLineGetFlag (Package, L"/b")), RetVal); ShellStatus = SHELL_SUCCESS; } + } else { + // If we are in a batch file and /b then pass TRUE otherwise false... + // + ShellCommandRegisterExit ((BOOLEAN)(gEfiShellProtocol->BatchIsActive () && ShellCommandLineGetFlag (Package, L"/b")), 0); - ShellCommandLineFreeVarList (Package); + ShellStatus = SHELL_SUCCESS; } + ShellCommandLineFreeVarList (Package); + return (ShellStatus); } diff --git a/ShellPkg/Library/UefiShellLevel1CommandsLib/Goto.c b/ShellPkg/Library/UefiShellLevel1CommandsLib/Goto.c index 408f2ff21b..d1495a0404 100644 --- a/ShellPkg/Library/UefiShellLevel1CommandsLib/Goto.c +++ b/ShellPkg/Library/UefiShellLevel1CommandsLib/Goto.c @@ -59,45 +59,47 @@ ShellCommandRunGoto ( } else { ASSERT (FALSE); } + + return ShellStatus; + } + + if (ShellCommandLineGetRawValue (Package, 2) != NULL) { + ShellPrintHiiDefaultEx (STRING_TOKEN (STR_GEN_TOO_MANY), gShellLevel1HiiHandle, L"goto"); + ShellStatus = SHELL_INVALID_PARAMETER; + } else if (ShellCommandLineGetRawValue (Package, 1) == NULL) { + ShellPrintHiiDefaultEx (STRING_TOKEN (STR_GEN_TOO_FEW), gShellLevel1HiiHandle, L"goto"); + ShellStatus = SHELL_INVALID_PARAMETER; } else { - if (ShellCommandLineGetRawValue (Package, 2) != NULL) { - ShellPrintHiiDefaultEx (STRING_TOKEN (STR_GEN_TOO_MANY), gShellLevel1HiiHandle, L"goto"); - ShellStatus = SHELL_INVALID_PARAMETER; - } else if (ShellCommandLineGetRawValue (Package, 1) == NULL) { - ShellPrintHiiDefaultEx (STRING_TOKEN (STR_GEN_TOO_FEW), gShellLevel1HiiHandle, L"goto"); - ShellStatus = SHELL_INVALID_PARAMETER; - } else { - Size = 0; - ASSERT ((CompareString == NULL && Size == 0) || (CompareString != NULL)); - CompareString = StrnCatGrow (&CompareString, &Size, L":", 0); - CompareString = StrnCatGrow (&CompareString, &Size, ShellCommandLineGetRawValue (Package, 1), 0); - if (CompareString == NULL) { - ShellCommandLineFreeVarList (Package); - return SHELL_OUT_OF_RESOURCES; - } - - // - // Check forwards and then backwards for a label... - // - if (!MoveToTag (GetNextNode, L"endfor", L"for", CompareString, ShellCommandGetCurrentScriptFile (), FALSE, FALSE, TRUE)) { - CurrentScriptFile = ShellCommandGetCurrentScriptFile (); - ShellPrintHiiDefaultEx ( - STRING_TOKEN (STR_SYNTAX_NO_MATCHING), - gShellLevel1HiiHandle, - CompareString, - L"Goto", - CurrentScriptFile != NULL - && CurrentScriptFile->CurrentCommand != NULL - ? CurrentScriptFile->CurrentCommand->Line : 0 - ); - ShellStatus = SHELL_NOT_FOUND; - } - - FreePool (CompareString); + Size = 0; + ASSERT ((CompareString == NULL && Size == 0) || (CompareString != NULL)); + CompareString = StrnCatGrow (&CompareString, &Size, L":", 0); + CompareString = StrnCatGrow (&CompareString, &Size, ShellCommandLineGetRawValue (Package, 1), 0); + if (CompareString == NULL) { + ShellCommandLineFreeVarList (Package); + return SHELL_OUT_OF_RESOURCES; } - ShellCommandLineFreeVarList (Package); + // + // Check forwards and then backwards for a label... + // + if (!MoveToTag (GetNextNode, L"endfor", L"for", CompareString, ShellCommandGetCurrentScriptFile (), FALSE, FALSE, TRUE)) { + CurrentScriptFile = ShellCommandGetCurrentScriptFile (); + ShellPrintHiiDefaultEx ( + STRING_TOKEN (STR_SYNTAX_NO_MATCHING), + gShellLevel1HiiHandle, + CompareString, + L"Goto", + CurrentScriptFile != NULL + && CurrentScriptFile->CurrentCommand != NULL + ? CurrentScriptFile->CurrentCommand->Line : 0 + ); + ShellStatus = SHELL_NOT_FOUND; + } + + FreePool (CompareString); } + ShellCommandLineFreeVarList (Package); + return (ShellStatus); } diff --git a/ShellPkg/Library/UefiShellLevel1CommandsLib/Stall.c b/ShellPkg/Library/UefiShellLevel1CommandsLib/Stall.c index b634cbf4a5..719df668ea 100644 --- a/ShellPkg/Library/UefiShellLevel1CommandsLib/Stall.c +++ b/ShellPkg/Library/UefiShellLevel1CommandsLib/Stall.c @@ -51,29 +51,31 @@ ShellCommandRunStall ( } else { ASSERT (FALSE); } + + return ShellStatus; + } + + if (ShellCommandLineGetRawValue (Package, 2) != NULL) { + ShellPrintHiiDefaultEx (STRING_TOKEN (STR_GEN_TOO_MANY), gShellLevel1HiiHandle, L"stall"); + ShellStatus = SHELL_INVALID_PARAMETER; + } else if (ShellCommandLineGetRawValue (Package, 1) == NULL) { + ShellPrintHiiDefaultEx (STRING_TOKEN (STR_GEN_TOO_FEW), gShellLevel1HiiHandle, L"stall"); + ShellStatus = SHELL_INVALID_PARAMETER; } else { - if (ShellCommandLineGetRawValue (Package, 2) != NULL) { - ShellPrintHiiDefaultEx (STRING_TOKEN (STR_GEN_TOO_MANY), gShellLevel1HiiHandle, L"stall"); - ShellStatus = SHELL_INVALID_PARAMETER; - } else if (ShellCommandLineGetRawValue (Package, 1) == NULL) { - ShellPrintHiiDefaultEx (STRING_TOKEN (STR_GEN_TOO_FEW), gShellLevel1HiiHandle, L"stall"); + Status = ShellConvertStringToUint64 (ShellCommandLineGetRawValue (Package, 1), &Intermediate, FALSE, FALSE); + if (EFI_ERROR (Status) || (((UINT64)(UINTN)(Intermediate)) != Intermediate)) { + ShellPrintHiiDefaultEx (STRING_TOKEN (STR_GEN_PARAM_INV), gShellLevel1HiiHandle, L"stall", ShellCommandLineGetRawValue (Package, 1)); ShellStatus = SHELL_INVALID_PARAMETER; } else { - Status = ShellConvertStringToUint64 (ShellCommandLineGetRawValue (Package, 1), &Intermediate, FALSE, FALSE); - if (EFI_ERROR (Status) || (((UINT64)(UINTN)(Intermediate)) != Intermediate)) { - ShellPrintHiiDefaultEx (STRING_TOKEN (STR_GEN_PARAM_INV), gShellLevel1HiiHandle, L"stall", ShellCommandLineGetRawValue (Package, 1)); - ShellStatus = SHELL_INVALID_PARAMETER; - } else { - Status = gBS->Stall ((UINTN)Intermediate); - if (EFI_ERROR (Status)) { - ShellPrintHiiDefaultEx (STRING_TOKEN (STR_STALL_FAILED), gShellLevel1HiiHandle, L"stall"); - ShellStatus = SHELL_DEVICE_ERROR; - } + Status = gBS->Stall ((UINTN)Intermediate); + if (EFI_ERROR (Status)) { + ShellPrintHiiDefaultEx (STRING_TOKEN (STR_STALL_FAILED), gShellLevel1HiiHandle, L"stall"); + ShellStatus = SHELL_DEVICE_ERROR; } } - - ShellCommandLineFreeVarList (Package); } + ShellCommandLineFreeVarList (Package); + return (ShellStatus); }