> echo Hello World Would be > HelloWorld
From: Qiu, Shumin Sent: Tuesday, June 21, 2016 10:55 PM To: Carsey, Jaben; [email protected] Cc: Ni, Ruiyu Subject: RE: [PATCH] ShellPkg: Fix 'echo' cannot display the special characters correctly. The space char around the argv is striped. > echo Hello World Would be >echo HelloWorld -Shumin From: Carsey, Jaben Sent: Tuesday, June 21, 2016 10:50 PM To: Qiu, Shumin; [email protected]<mailto:[email protected]> Cc: Ni, Ruiyu; Carsey, Jaben Subject: RE: [PATCH] ShellPkg: Fix 'echo' cannot display the special characters correctly. Why? What are we losing in conversion to argc/argv... I thought maybe we should skip the Lib-based parsing of the parameters? Why does this not work: for (x=1;x<ShellParametersProtocol->Argc,x++) // skip 0 which is "echo" { print ShellParametersProtocol->Argv[x]; } From: Qiu, Shumin Sent: Tuesday, June 21, 2016 7:47 AM To: Carsey, Jaben <[email protected]<mailto:[email protected]>>; [email protected]<mailto:[email protected]> Cc: Ni, Ruiyu <[email protected]<mailto:[email protected]>> Subject: RE: [PATCH] ShellPkg: Fix 'echo' cannot display the special characters correctly. Importance: High Hi Jaben, 'echo' should not use ShellParameterProtocol to pass the parameters. It prints out the string following 'echo' literally. So we need pass the 'CmdLine' directly. -Shumin From: Carsey, Jaben Sent: Tuesday, June 21, 2016 10:14 PM To: Qiu, Shumin; [email protected]<mailto:[email protected]> Cc: Ni, Ruiyu; Carsey, Jaben Subject: RE: [PATCH] ShellPkg: Fix 'echo' cannot display the special characters correctly. I don't think that means we are not carrying around the size of the echo code. It looks like we still call the echo API directly through the RunInternalCommand() API. (copied below) Can you explain what was the change that required the function had to move to Shell.c from the Echo.c ? > @@ -2288,6 +2383,21 @@ RunInternalCommand( > } > } > > + if (gUnicodeCollation->StriColl( > + gUnicodeCollation, > + FirstParameter, > + L"echo") == 0) { > + Status = ShellInternalRunEcho (NewCmdLine); > + if (CommandStatus != NULL) { > + *CommandStatus = Status; From: Qiu, Shumin Sent: Monday, June 20, 2016 8:21 PM To: Carsey, Jaben <[email protected]<mailto:[email protected]>>; [email protected]<mailto:[email protected]> Cc: Ni, Ruiyu <[email protected]<mailto:[email protected]>> Subject: RE: [PATCH] ShellPkg: Fix 'echo' cannot display the special characters correctly. Importance: High Hi Jaben, If level 3 command is not elected the 'echo' node won't be added into 'SHELL_COMMAND_INTERNAL_LIST_ENTRY mCommandList' list. If the user run 'echo' Shell couldn't recognize this command. -Shumin From: Carsey, Jaben Sent: Monday, June 20, 2016 10:45 PM To: Qiu, Shumin; [email protected]<mailto:[email protected]> Cc: Ni, Ruiyu; Carsey, Jaben Subject: RE: [PATCH] ShellPkg: Fix 'echo' cannot display the special characters correctly. I think we need to revisit this. If the user elects to build a shell that does not contain level 3 commands, this looks like the echo command would still exist and function which would violate the spec. -Jaben > -----Original Message----- > From: Qiu, Shumin > Sent: Monday, June 20, 2016 12:30 AM > To: [email protected]<mailto:[email protected]> > Cc: Qiu, Shumin <[email protected]<mailto:[email protected]>>; Carsey, > Jaben > <[email protected]<mailto:[email protected]>>; Ni, Ruiyu > <[email protected]<mailto:[email protected]>> > Subject: [PATCH] ShellPkg: Fix 'echo' cannot display the special > characters correctly. > Importance: High > > Currently run 'echo -t' will get the result: > echo: Unknown flag - '-t' > The expected result is to display '-t' literally. > This patch adds special handle for 'echo'. 'echo' will not use the > general parameter parsing library . > > Cc: Jaben Carsey <[email protected]<mailto:[email protected]>> > Cc: Ruiyu Ni <[email protected]<mailto:[email protected]>> > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Qiu Shumin <[email protected]<mailto:[email protected]>> > --- > ShellPkg/Application/Shell/Shell.c | 110 > +++++++++++++++++++++ > ShellPkg/Application/Shell/Shell.uni | 4 + > ShellPkg/Library/UefiShellLevel3CommandsLib/Echo.c | 89 > +---------------- > 3 files changed, 117 insertions(+), 86 deletions(-) > > diff --git a/ShellPkg/Application/Shell/Shell.c > b/ShellPkg/Application/Shell/Shell.c > index 54ca76a..fa75b19 100644 > --- a/ShellPkg/Application/Shell/Shell.c > +++ b/ShellPkg/Application/Shell/Shell.c > @@ -105,6 +105,101 @@ TrimSpaces( > } > > /** > + Function for 'echo' command. > + > + @param[in] CmdLine the command line to run. > +**/ > +EFI_STATUS > +EFIAPI > +ShellInternalRunEcho( > + IN CONST CHAR16 *CmdLine > + ) > +{ > + EFI_STATUS Status; > + UINTN Size; > + CHAR16 *PrintString; > + CHAR16 *NewCommandLine; > + CHAR16 *Walker; > + CHAR16 *TempParameter; > + BOOLEAN OnFlag; > + BOOLEAN OffFlag; > + UINTN Count; > + > + Size = 0; > + PrintString = NULL; > + NewCommandLine = NULL; > + TempParameter = NULL; > + Status = EFI_SUCCESS; > + OnFlag = FALSE; > + OffFlag = FALSE; > + > + // > + // parse the command line > + // > + Size = StrSize(CmdLine); > + NewCommandLine = AllocateCopyPool(Size, CmdLine); ASSERT > + (NewCommandLine != NULL); TempParameter = AllocateZeroPool(Size); > + ASSERT (TempParameter != NULL); > + > + > + for ( Count = 0 > + , Walker = (CHAR16*)NewCommandLine > + ; Walker != NULL && *Walker != CHAR_NULL > + ; Count++ > + ) { > + if (EFI_ERROR(GetNextParameter(&Walker, &TempParameter, Size, > FALSE))) { > + break; > + } > + > + if (Count == 1) { > + if (gUnicodeCollation->StriColl(gUnicodeCollation, > + TempParameter, L"- > on") == 0 ) { > + OnFlag = TRUE; > + } > + if (gUnicodeCollation->StriColl(gUnicodeCollation, > + TempParameter, L"- > off") == 0 ) { > + OffFlag = TRUE; > + } > + } > + } > + > + if ((OnFlag || OffFlag) && Count != 2 ) { > + ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN > (STR_ECHO_INVALID_PARAM), ShellInfoObject.HiiHandle, L"echo", L"-on/- > off"); > + Status = EFI_INVALID_PARAMETER; > + goto Done; > + } > + > + if (OnFlag) { > + ShellCommandSetEchoState(TRUE); > + Status = EFI_SUCCESS; > + goto Done; > + } else if (OffFlag) { > + ShellCommandSetEchoState(FALSE); > + Status = EFI_SUCCESS; > + goto Done; > + } > + > + Walker = NewCommandLine + StrLen(L"echo"); if (StrLen (Walker) <= > + 1) { > + Status = EFI_SUCCESS; > + goto Done; > + } else { > + Walker++; > + PrintString = AllocateCopyPool(StrSize(Walker), Walker); > + ASSERT (PrintString != NULL); > + ShellPrintEx(-1, -1, L"%s\r\n", PrintString); } > + > + Status = EFI_SUCCESS; > + > +Done: > + SHELL_FREE_NON_NULL(NewCommandLine); > + SHELL_FREE_NON_NULL(TempParameter); > + SHELL_FREE_NON_NULL(PrintString); > + return (Status); > +} > + > + > +/** > Parse for the next instance of one string within another string. > Can optionally make sure that > the string was not escaped (^ character) per the shell specification. > > @@ -2288,6 +2383,21 @@ RunInternalCommand( > } > } > > + if (gUnicodeCollation->StriColl( > + gUnicodeCollation, > + FirstParameter, > + L"echo") == 0) { > + Status = ShellInternalRunEcho (NewCmdLine); > + if (CommandStatus != NULL) { > + *CommandStatus = Status; > + } > + FreePool (NewCmdLine); > + if (EFI_ERROR(Status)) { > + Status = EFI_ABORTED; > + } > + return Status; > + } > + > // > // get the argc and argv updated for internal commands > // > diff --git a/ShellPkg/Application/Shell/Shell.uni > b/ShellPkg/Application/Shell/Shell.uni > index ef69f89..6920a9a 100644 > --- a/ShellPkg/Application/Shell/Shell.uni > +++ b/ShellPkg/Application/Shell/Shell.uni > @@ -56,3 +56,7 @@ > > #string STR_SHELL_IMAGE_NOT_APP #language en-US "The image is not > an application.\r\n" > > +#string STR_ECHO_INVALID_PARAM #language en-US "%H%s%N: > Invalid - too many parameters after '%s'\r\n" > + > +#string STR_ECHO_TOO_FEW_PARAM #language en-US "%H%s%N: > Invalid - too few parameters\r\n > + > diff --git a/ShellPkg/Library/UefiShellLevel3CommandsLib/Echo.c > b/ShellPkg/Library/UefiShellLevel3CommandsLib/Echo.c > index a638de8..71001d6 100644 > --- a/ShellPkg/Library/UefiShellLevel3CommandsLib/Echo.c > +++ b/ShellPkg/Library/UefiShellLevel3CommandsLib/Echo.c > @@ -2,7 +2,7 @@ > Main file for Echo shell level 3 function. > > (C) Copyright 2015 Hewlett-Packard Development Company, L.P.<BR> > - Copyright (c) 2009 - 2012, Intel Corporation. All rights reserved. > <BR> > + Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved. > + <BR> > This program and the accompanying materials > are licensed and made available under the terms and conditions of > the BSD License > which accompanies this distribution. The full text of the license > may be found at @@ -17,12 +17,6 @@ > > #include <Library/ShellLib.h> > > -STATIC CONST SHELL_PARAM_ITEM ParamList[] = { > - {L"-on", TypeFlag}, > - {L"-off", TypeFlag}, > - {NULL, TypeMax} > - }; > - > /** > Function for 'echo' command. > > @@ -36,86 +30,9 @@ ShellCommandRunEcho ( > IN EFI_SYSTEM_TABLE *SystemTable > ) > { > - EFI_STATUS Status; > - LIST_ENTRY *Package; > - SHELL_STATUS ShellStatus; > - UINTN ParamCount; > - CHAR16 *ProblemParam; > - UINTN Size; > - CHAR16 *PrintString; > - > - Size = 0; > - ProblemParam = NULL; > - PrintString = NULL; > - ShellStatus = SHELL_SUCCESS; > - > - // > - // initialize the shell lib (we must be in non-auto-init...) > // > - Status = ShellInitialize(); > - ASSERT_EFI_ERROR(Status); > - > - // > - // parse the command line > + // Echo command are handled in ShellInternalRunEcho <Shell.c> > // > - Status = ShellCommandLineParseEx (ParamList, &Package, > &ProblemParam, TRUE, TRUE); > - if (EFI_ERROR(Status)) { > - if (Status == EFI_VOLUME_CORRUPTED && ProblemParam != NULL) { > - ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN (STR_GEN_PROBLEM), > gShellLevel3HiiHandle, L"echo", ProblemParam); > - FreePool(ProblemParam); > - ShellStatus = SHELL_INVALID_PARAMETER; > - } else { > - ASSERT(FALSE); > - } > - } else { > - // > - // check for "-?" > - // > - if (ShellCommandLineGetFlag(Package, L"-?")) { > - ASSERT(FALSE); > - } > - if (ShellCommandLineGetFlag(Package, L"-on")) { > - // > - // Turn it on > - // > - ShellCommandSetEchoState(TRUE); > - } else if (ShellCommandLineGetFlag(Package, L"-off")) { > - // > - // turn it off > - // > - ShellCommandSetEchoState(FALSE); > - } else if (ShellCommandLineGetRawValue(Package, 1) == NULL) { > - // > - // output its current state > - // > - if (ShellCommandGetEchoState()) { > - ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN (STR_ECHO_ON), > gShellLevel3HiiHandle); > - } else { > - ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN (STR_ECHO_OFF), > gShellLevel3HiiHandle); > - } > - } else { > - // > - // print the line > - // > - for ( ParamCount = 1 > - ; ShellCommandLineGetRawValue(Package, ParamCount) != NULL > - ; ParamCount++ > - ) { > - StrnCatGrow(&PrintString, &Size, > ShellCommandLineGetRawValue(Package, ParamCount), 0); > - if (ShellCommandLineGetRawValue(Package, ParamCount+1) != NULL) { > - StrnCatGrow(&PrintString, &Size, L" ", 0); > - } > - } > - ShellPrintEx(-1, -1, L"%s\r\n", PrintString); > - SHELL_FREE_NON_NULL(PrintString); > - } > - > - // > - // free the command line package > - // > - ShellCommandLineFreeVarList (Package); > - } > - > - return (ShellStatus); > + return (SHELL_SUCCESS); > } > > -- > 2.7.1.windows.2 _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel

