Re: [PATCH] ShellPkg/parse: Handle Unicode stream from pipe correctly

["Shah, Tapan" <tapandshah@hpe.com>]

Reviewed-by: Tapan Shah <tapandshah@hpe.com>

-----Original Message-----
From: Ni, Ruiyu [mailto:ruiyu.ni@intel.com] 
Sent: Tuesday, June 06, 2017 3:44 AM
To: Ni, Ruiyu <ruiyu.ni@intel.com>; edk2-devel@lists.01.org
Cc: Shah, Tapan <tapandshah@hpe.com>
Subject: RE: [edk2] [PATCH] ShellPkg/parse: Handle Unicode stream from pipe correctly

Including Tapan.

Thanks/Ray

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of 
> Ruiyu Ni
> Sent: Tuesday, June 6, 2017 4:29 PM
> To: edk2-devel@lists.01.org
> Subject: [edk2] [PATCH] ShellPkg/parse: Handle Unicode stream from 
> pipe correctly
> 
> The original code expects the Unicode stream from pipe doesn't 
> contains the Unicode BOM.
> But that's not true.
> Commit [9ed21946c76e430097e9c4e59b419af928e0cb8c] changes
> CreateFileInterfaceMem() to add the BOM for Unicode stream.
> 
> When parse pipe support was firstly added, a private implementation
> ParseReturnStdInLine() was created to specially handle the Unicode 
> stream without BOM. Since now the Unicode steam contains BOM, the 
> private implementation can be removed and
> ShellFileHandleReturnLine() can be used directly.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Tapan Shah <tapandshah@hpe.com>
> ---
>  .../Library/UefiShellLevel2CommandsLib/Parse.c     | 145 +--------------------
>  1 file changed, 3 insertions(+), 142 deletions(-)
> 
> diff --git a/ShellPkg/Library/UefiShellLevel2CommandsLib/Parse.c
> b/ShellPkg/Library/UefiShellLevel2CommandsLib/Parse.c
> index 4b1973a505..85c39ba78f 100644
> --- a/ShellPkg/Library/UefiShellLevel2CommandsLib/Parse.c
> +++ b/ShellPkg/Library/UefiShellLevel2CommandsLib/Parse.c
> @@ -2,7 +2,7 @@
>    Main file for Parse shell level 2 function.
> 
>    (C) Copyright 2013-2015 Hewlett-Packard Development Company, 
> L.P.<BR>
> -  Copyright (c) 2009 - 2012, Intel Corporation. All rights 
> reserved.<BR>
> +  Copyright (c) 2009 - 2017, 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 @@ -56,137 +56,6 @@ IsStdInDataAvailable (  }
> 
>  /**
> -  Function to read a single line (up to but not including the \n) 
> using StdIn data from a SHELL_FILE_HANDLE.
> -
> -  If the position upon start is 0, then the Ascii Boolean will be 
> set.  This should be
> -  maintained and not changed for all operations with the same file.
> -
> -  @param[in]       Handle        SHELL_FILE_HANDLE to read from.
> -  @param[in, out]  Buffer        The pointer to buffer to read into.
> -  @param[in, out]  Size          The pointer to number of bytes in Buffer.
> -  @param[in]       Truncate      If the buffer is large enough, this has no effect.
> -                                 If the buffer is is too small and Truncate is TRUE,
> -                                 the line will be truncated.
> -                                 If the buffer is is too small and Truncate is FALSE,
> -                                 then no read will occur.
> -
> -  @retval EFI_SUCCESS           The operation was successful.  The line is stored in
> -                                Buffer.
> -  @retval EFI_INVALID_PARAMETER Handle was NULL.
> -  @retval EFI_INVALID_PARAMETER Size was NULL.
> -  @retval EFI_BUFFER_TOO_SMALL  Size was not large enough to store the line.
> -                                Size was updated to the minimum space required.
> -**/
> -EFI_STATUS
> -ShellFileHandleReadStdInLine(
> -  IN SHELL_FILE_HANDLE          Handle,
> -  IN OUT CHAR16                 *Buffer,
> -  IN OUT UINTN                  *Size,
> -  IN BOOLEAN                    Truncate
> -  )
> -{
> -  EFI_STATUS  Status;
> -  CHAR16      CharBuffer;
> -  UINTN       CharSize;
> -  UINTN       CountSoFar;
> -  UINT64      OriginalFilePosition;
> -
> -
> -  if (Handle == NULL
> -    ||Size   == NULL
> -   ){
> -    return (EFI_INVALID_PARAMETER);
> -  }
> -  if (Buffer == NULL) {
> -    ASSERT(*Size == 0);
> -  } else {
> -    *Buffer = CHAR_NULL;
> -  }
> -  gEfiShellProtocol->GetFilePosition (Handle, &OriginalFilePosition);
> -
> -  for (CountSoFar = 0;;CountSoFar++){
> -    CharBuffer = 0;
> -    CharSize = sizeof(CHAR16);
> -    Status = gEfiShellProtocol->ReadFile (Handle, &CharSize, &CharBuffer);
> -    if (  EFI_ERROR(Status)
> -       || CharSize == 0
> -       || (CharBuffer == L'\n')
> -     ){
> -      break;
> -    }
> -    //
> -    // if we have space save it...
> -    //
> -    if ((CountSoFar+1)*sizeof(CHAR16) < *Size){
> -      ASSERT(Buffer != NULL);
> -      ((CHAR16*)Buffer)[CountSoFar] = CharBuffer;
> -      ((CHAR16*)Buffer)[CountSoFar+1] = CHAR_NULL;
> -    }
> -  }
> -
> -  //
> -  // if we ran out of space tell when...
> -  //
> -  if ((CountSoFar+1)*sizeof(CHAR16) > *Size){
> -    *Size = (CountSoFar+1)*sizeof(CHAR16);
> -    if (!Truncate) {
> -      gEfiShellProtocol->SetFilePosition(Handle, OriginalFilePosition);
> -    } else {
> -      DEBUG((DEBUG_WARN, "The line was truncated in
> ShellFileHandleReadLine"));
> -    }
> -    return (EFI_BUFFER_TOO_SMALL);
> -  }
> -  while(Buffer[StrLen(Buffer)-1] == L'\r') {
> -    Buffer[StrLen(Buffer)-1] = CHAR_NULL;
> -  }
> -
> -  return (Status);
> -}
> -
> -
> -/**
> -  Function to read a single line using StdIn from a 
> SHELL_FILE_HANDLE. The \n is not included in the returned
> -  buffer.  The returned buffer must be callee freed.
> -
> -  If the position upon start is 0, then the Ascii Boolean will be 
> set.  This should be
> -  maintained and not changed for all operations with the same file.
> -
> -  @param[in]       Handle        SHELL_FILE_HANDLE to read from.
> -
> -  @return                        The line of text from the file.
> -  @retval NULL                   There was not enough memory available.
> -
> -  @sa ShellFileHandleReadLine
> -**/
> -CHAR16*
> -ParseReturnStdInLine (
> -  IN SHELL_FILE_HANDLE Handle
> -  )
> -{
> -  CHAR16          *RetVal;
> -  UINTN           Size;
> -  EFI_STATUS      Status;
> -
> -  Size   = 0;
> -  RetVal = NULL;
> -
> -  Status = ShellFileHandleReadStdInLine (Handle, RetVal, &Size, 
> FALSE);
> -  if (Status == EFI_BUFFER_TOO_SMALL) {
> -    RetVal = AllocateZeroPool(Size);
> -    if (RetVal == NULL) {
> -      return (NULL);
> -    }
> -    Status = ShellFileHandleReadStdInLine (Handle, RetVal, &Size, FALSE);
> -
> -  }
> -  if (EFI_ERROR(Status) && (RetVal != NULL)) {
> -    FreePool(RetVal);
> -    RetVal = NULL;
> -  }
> -  return (RetVal);
> -}
> -
> -/**
>    Handle stings for SFO Output with escape character ^ in a string
>    1. Quotation marks in the string must be escaped by using a ^ character (i.e.
> ^").
>    2. The ^ character may be inserted using ^^.
> @@ -281,11 +150,7 @@ PerformParsing(
>      ShellStatus = SHELL_NOT_FOUND;
>    } else {
>      for (LoopVariable = 0 ; LoopVariable < ShellCommandInstance && 
> !ShellFileHandleEof(FileHandle);) {
> -     if (StreamingUnicode) {
> -       TempLine = ParseReturnStdInLine (FileHandle);
> -     } else {
> -       TempLine = ShellFileHandleReturnLine (FileHandle, &Ascii);
> -     }
> +     TempLine = ShellFileHandleReturnLine (FileHandle, &Ascii);
> 
>        if ((TempLine == NULL) || (*TempLine == CHAR_NULL &&
> StreamingUnicode)) {
>           break;
> @@ -304,11 +169,7 @@ PerformParsing(
>      if (LoopVariable == ShellCommandInstance) {
>        LoopVariable = 0;
>        while(1) {
> -        if (StreamingUnicode) {
> -          TempLine = ParseReturnStdInLine (FileHandle);
> -        } else {
> -          TempLine = ShellFileHandleReturnLine (FileHandle, &Ascii);
> -        }
> +        TempLine = ShellFileHandleReturnLine (FileHandle, &Ascii);
>          if (TempLine == NULL
>              || *TempLine == CHAR_NULL
>              || StrStr (TempLine, L"ShellCommand,") == TempLine) {
> --
> 2.12.2.windows.2
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel