From: "Carsey, Jaben" <jaben.carsey@intel.com>
To: "Ni, Ruiyu" <ruiyu.ni@intel.com>,
"Ni, Ruiyu" <ruiyu.ni@intel.com>,
"edk2-devel@lists.01.org" <edk2-devel@lists.01.org>
Subject: Re: [PATCH] ShellPkg/parse: Handle Unicode stream from pipe correctly
Date: Tue, 6 Jun 2017 15:41:52 +0000 [thread overview]
Message-ID: <CB6E33457884FA40993F35157061515C6CAA7C9D@FMSMSX103.amr.corp.intel.com> (raw)
In-Reply-To: <734D49CCEBEEF84792F5B80ED585239D5B988FCE@SHSMSX104.ccr.corp.intel.com>
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
next prev parent reply other threads:[~2017-06-06 15:40 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
2017-06-06 22:05 ` Shah, Tapan
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CB6E33457884FA40993F35157061515C6CAA7C9D@FMSMSX103.amr.corp.intel.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox