public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH] ShellPkg/parse: Handle Unicode stream from pipe correctly
@ 2017-06-06  8:29 Ruiyu Ni
  2017-06-06  8:43 ` Ni, Ruiyu
  0 siblings, 1 reply; 4+ messages in thread
From: Ruiyu Ni @ 2017-06-06  8:29 UTC (permalink / raw)
  To: edk2-devel

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



^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] ShellPkg/parse: Handle Unicode stream from pipe correctly
  2017-06-06  8:29 [PATCH] ShellPkg/parse: Handle Unicode stream from pipe correctly Ruiyu Ni
@ 2017-06-06  8:43 ` Ni, Ruiyu
  2017-06-06 15:41   ` Carsey, Jaben
  2017-06-06 22:05   ` Shah, Tapan
  0 siblings, 2 replies; 4+ messages in thread
From: Ni, Ruiyu @ 2017-06-06  8:43 UTC (permalink / raw)
  To: Ni, Ruiyu, edk2-devel@lists.01.org

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


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] ShellPkg/parse: Handle Unicode stream from pipe correctly
  2017-06-06  8:43 ` Ni, Ruiyu
@ 2017-06-06 15:41   ` Carsey, Jaben
  2017-06-06 22:05   ` Shah, Tapan
  1 sibling, 0 replies; 4+ messages in thread
From: Carsey, Jaben @ 2017-06-06 15:41 UTC (permalink / raw)
  To: Ni, Ruiyu, Ni, Ruiyu, edk2-devel@lists.01.org

I like removing the special local version of the function.
Reviewed-by: Jaben Carsey <jaben.carsey@intel.com>

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ni,
> Ruiyu
> Sent: Tuesday, June 06, 2017 1:44 AM
> To: Ni, Ruiyu <ruiyu.ni@intel.com>; edk2-devel@lists.01.org
> Subject: Re: [edk2] [PATCH] ShellPkg/parse: Handle Unicode stream from
> pipe correctly
> Importance: High
> 
> 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
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] ShellPkg/parse: Handle Unicode stream from pipe correctly
  2017-06-06  8:43 ` Ni, Ruiyu
  2017-06-06 15:41   ` Carsey, Jaben
@ 2017-06-06 22:05   ` Shah, Tapan
  1 sibling, 0 replies; 4+ messages in thread
From: Shah, Tapan @ 2017-06-06 22:05 UTC (permalink / raw)
  To: Ni, Ruiyu, edk2-devel@lists.01.org

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


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2017-06-06 22:04 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-06-06  8:29 [PATCH] ShellPkg/parse: Handle Unicode stream from pipe correctly Ruiyu Ni
2017-06-06  8:43 ` Ni, Ruiyu
2017-06-06 15:41   ` Carsey, Jaben
2017-06-06 22:05   ` Shah, Tapan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox