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]
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

Reply via email to