public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Ni, Ruiyu" <ruiyu.ni@intel.com>
To: "Ni, Ruiyu" <ruiyu.ni@intel.com>,
	"edk2-devel@lists.01.org" <edk2-devel@lists.01.org>
Cc: "Carsey, Jaben" <jaben.carsey@intel.com>
Subject: Re: [PATCH v2 1/6] ShellPkg: TAB logic incorrectly chops out fs0: when typing fs0:<TAB>
Date: Tue, 9 Aug 2016 05:44:33 +0000	[thread overview]
Message-ID: <734D49CCEBEEF84792F5B80ED585239D1ADB4A20@SHSMSX103.ccr.corp.intel.com> (raw)
In-Reply-To: <20160809054338.170452-1-ruiyu.ni@intel.com>

This v2 patch fixed VS2012 build failure.

Thanks/Ray

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Ruiyu Ni
> Sent: Tuesday, August 9, 2016 1:44 PM
> To: edk2-devel@lists.01.org
> Cc: Carsey, Jaben <jaben.carsey@intel.com>
> Subject: [edk2] [PATCH v2 1/6] ShellPkg: TAB logic incorrectly chops out fs0:
> when typing fs0:<TAB>
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Jaben Carsey <jaben.carsey@intel.com>
> ---
>  ShellPkg/Application/Shell/FileHandleWrappers.c | 312 ++++++++++++++---
> -------
>  1 file changed, 185 insertions(+), 127 deletions(-)
> 
> diff --git a/ShellPkg/Application/Shell/FileHandleWrappers.c
> b/ShellPkg/Application/Shell/FileHandleWrappers.c
> index f6a82ee..0091934 100644
> --- a/ShellPkg/Application/Shell/FileHandleWrappers.c
> +++ b/ShellPkg/Application/Shell/FileHandleWrappers.c
> @@ -293,6 +293,134 @@ FileInterfaceNulWrite(  }
> 
>  /**
> +  Create the TAB completion list.
> +
> +  @param[in]  InputString       The command line to expand.
> +  @param[in]  StringLen         Length of the command line.
> +  @param[in]  BufferSize        Buffer size.
> +  @param[out] TabCompletionList Return the TAB completion list.
> +  @param[out] TabUpdatePos      Return the TAB update position.
> +**/
> +EFI_STATUS
> +EFIAPI
> +CreateTabCompletionList (
> +  IN CONST CHAR16             *InputString,
> +  IN CONST UINTN              StringLen,
> +  IN CONST UINTN              BufferSize,
> +  IN OUT EFI_SHELL_FILE_INFO  **TabCompletionList,
> +  IN OUT   UINTN              *TabUpdatePos
> +)
> +{
> +  BOOLEAN             InQuotation;
> +  UINTN               TabPos;
> +  UINTN               Index;
> +  CONST CHAR16        *Cwd;
> +  EFI_STATUS          Status;
> +  CHAR16              *TabStr;
> +  EFI_SHELL_FILE_INFO *FileList;
> +  EFI_SHELL_FILE_INFO *FileInfo;
> +  EFI_SHELL_FILE_INFO *TempFileInfo;
> +
> +  //
> +  // Allocate buffers
> +  //
> +  TabStr = AllocateZeroPool (BufferSize);  if (TabStr == NULL) {
> +    return EFI_OUT_OF_RESOURCES;
> +  }
> +
> +  //
> +  // handle auto complete of file and directory names...
> +  // E.g.: cd fs0:\EFI\Bo<TAB>
> +  //          ^        ^
> +  //          TabPos   TabUpdatePos
> +  //
> +  TabPos        = 0;
> +  *TabUpdatePos = 0;
> +  FileList      = NULL;
> +  InQuotation   = FALSE;
> +  for (Index = 0; Index < StringLen; Index++) {
> +    switch (InputString[Index]) {
> +    case L'\"':
> +      InQuotation = (BOOLEAN) (!InQuotation);
> +      break;
> +
> +    case L' ':
> +      if (!InQuotation) {
> +        TabPos = Index + 1;
> +        *TabUpdatePos = TabPos;
> +      }
> +      break;
> +
> +    case L':':
> +      //
> +      // handle the case "fs0:<TAB>"
> +      // Update the TabUpdatePos as well.
> +      //
> +    case L'\\':
> +      *TabUpdatePos = Index + 1;
> +      break;
> +
> +    default:
> +      break;
> +    }
> +  }
> +
> +  if (StrStr (InputString + TabPos, L":") == NULL) {
> +    //
> +    // If file path doesn't contain ":", it's a path relative to current directory.
> +    //
> +    Cwd = ShellInfoObject.NewEfiShellProtocol->GetCurDir (NULL);
> +    if (Cwd != NULL) {
> +      StrnCpyS (TabStr, (BufferSize) / sizeof (CHAR16), Cwd, (BufferSize) /
> sizeof (CHAR16) - 1);
> +      if (InputString[TabPos] != L'\\') {
> +        StrCatS (TabStr, (BufferSize) / sizeof (CHAR16), L"\\");
> +      }
> +    }
> +  }
> +  StrnCatS (TabStr, (BufferSize) / sizeof (CHAR16), InputString +
> + TabPos, StringLen - TabPos);  StrnCatS (TabStr, (BufferSize) / sizeof
> + (CHAR16), L"*", (BufferSize) / sizeof (CHAR16) - 1 - StrLen (TabStr));
> + Status  = ShellInfoObject.NewEfiShellProtocol->FindFiles(TabStr,
> + &FileList);
> +
> +  //
> +  // Filter out the non-directory for "CD" command  // Filter "." and
> + ".." for all  //  if (!EFI_ERROR (Status) && FileList != NULL) {
> +    //
> +    // Skip the spaces in the beginning
> +    //
> +    while (*InputString == L' ') {
> +      InputString++;
> +    }
> +
> +    for (FileInfo = (EFI_SHELL_FILE_INFO *) GetFirstNode (&FileList-
> >Link); !IsNull (&FileList->Link, &FileInfo->Link); ) {
> +      if (((StrCmp (FileInfo->FileName, L".") == 0) || (StrCmp (FileInfo-
> >FileName, L"..") == 0)) ||
> +          (((InputString[0] == L'c' || InputString[0] == L'C') && (InputString[1] ==
> L'd' || InputString[1] == L'D')) &&
> +           (ShellIsDirectory (FileInfo->FullName) != EFI_SUCCESS))) {
> +        TempFileInfo = FileInfo;
> +        FileInfo = (EFI_SHELL_FILE_INFO *) RemoveEntryList (&FileInfo->Link);
> +        InternalFreeShellFileInfoNode (TempFileInfo);
> +      } else {
> +        FileInfo = (EFI_SHELL_FILE_INFO *) GetNextNode (&FileList->Link,
> &FileInfo->Link);
> +      }
> +    }
> +  }
> +
> +  if (FileList != NULL && !IsListEmpty (&FileList->Link)) {
> +    Status = EFI_SUCCESS;
> +  } else {
> +    ShellInfoObject.NewEfiShellProtocol->FreeFileList (&FileList);
> +    Status = EFI_NOT_FOUND;
> +  }
> +
> +  FreePool (TabStr);
> +
> +  *TabCompletionList = FileList;
> +  return Status;
> +}
> +
> +/**
>    File style interface for console (Read).
> 
>    This will return a single line of input from the console.
> @@ -326,6 +454,7 @@ FileInterfaceStdInRead(  {
>    CHAR16              *CurrentString;
>    BOOLEAN             Done;
> +  UINTN               TabUpdatePos;   // Start index of the string updated by TAB
> stroke
>    UINTN               Column;         // Column of current cursor
>    UINTN               Row;            // Row of current cursor
>    UINTN               StartColumn;    // Column at the beginning of the line
> @@ -334,7 +463,6 @@ FileInterfaceStdInRead(
>    UINTN               StringLen;      // Total length of the line
>    UINTN               StringCurPos;   // Line index corresponding to the cursor
>    UINTN               MaxStr;         // Maximum possible line length
> -  UINTN               Index;
>    UINTN               TotalColumn;     // Num of columns in the console
>    UINTN               TotalRow;       // Num of rows in the console
>    UINTN               SkipLength;
> @@ -348,18 +476,10 @@ FileInterfaceStdInRead(
>    BOOLEAN             InScrolling;
>    EFI_STATUS          Status;
>    BOOLEAN             InTabScrolling; // Whether in TAB-completion state
> -  EFI_SHELL_FILE_INFO *FoundFileList;
> -  EFI_SHELL_FILE_INFO *TabLinePos;
> -  EFI_SHELL_FILE_INFO *TempPos;
> -  CHAR16              *TabStr;
> -  CHAR16              *TabOutputStr;
> -  BOOLEAN             InQuotationMode;
> -  CHAR16              *TempStr;
> -  UINTN               TabPos;         // Start index of the string to search for TAB
> completion.
> -  UINTN               TabUpdatePos;   // Start index of the string updated by TAB
> stroke
> -//  UINTN               Count;
> +  EFI_SHELL_FILE_INFO *TabCompleteList;  EFI_SHELL_FILE_INFO
> + *TabCurrent;
>    UINTN               EventIndex;
> -  CONST CHAR16        *Cwd;
> +  CHAR16              *TabOutputStr;
> 
>    //
>    // If buffer is not large enough to hold a CHAR16, return minimum buffer
> size @@ -380,24 +500,10 @@ FileInterfaceStdInRead(
>    InScrolling       = FALSE;
>    InTabScrolling    = FALSE;
>    Status            = EFI_SUCCESS;
> -  TabLinePos        = NULL;
> -  FoundFileList     = NULL;
> -  TempPos           = NULL;
> -  TabPos            = 0;
> +  TabOutputStr      = NULL;
>    TabUpdatePos      = 0;
> -
> -  //
> -  // Allocate buffers
> -  //
> -  TabStr            = AllocateZeroPool (*BufferSize);
> -  if (TabStr == NULL) {
> -    return EFI_OUT_OF_RESOURCES;
> -  }
> -  TabOutputStr      = AllocateZeroPool (*BufferSize);
> -  if (TabOutputStr == NULL) {
> -    FreePool(TabStr);
> -    return EFI_OUT_OF_RESOURCES;
> -  }
> +  TabCompleteList   = NULL;
> +  TabCurrent        = NULL;
> 
>    //
>    // Get the screen setting and the current cursor location @@ -454,11
> +560,11 @@ FileInterfaceStdInRead(
>      // If we are quitting TAB scrolling...
>      //
>      if (InTabScrolling && Key.UnicodeChar != CHAR_TAB) {
> -        if (FoundFileList != NULL) {
> -          ShellInfoObject.NewEfiShellProtocol->FreeFileList (&FoundFileList);
> -          DEBUG_CODE(FoundFileList = NULL;);
> -        }
> -        InTabScrolling = FALSE;
> +      if (TabCompleteList != NULL) {
> +        ShellInfoObject.NewEfiShellProtocol->FreeFileList (&TabCompleteList);
> +        DEBUG_CODE(TabCompleteList = NULL;);
> +      }
> +      InTabScrolling = FALSE;
>      }
> 
>      switch (Key.UnicodeChar) {
> @@ -491,95 +597,39 @@ FileInterfaceStdInRead(
>        break;
> 
>      case CHAR_TAB:
> -      //
> -      // handle auto complete of file and directory names...
> -      //
> -      if (InTabScrolling) {
> -        ASSERT(FoundFileList != NULL);
> -        ASSERT(TabLinePos != NULL);
> -        TabLinePos = (EFI_SHELL_FILE_INFO*)GetNextNode(&(FoundFileList-
> >Link), &TabLinePos->Link);
> -        if (IsNull(&(FoundFileList->Link), &TabLinePos->Link)) {
> -          TabLinePos = (EFI_SHELL_FILE_INFO*)GetNextNode(&(FoundFileList-
> >Link), &TabLinePos->Link);
> -        }
> -      } else {
> -        TabPos          = 0;
> -        TabUpdatePos    = 0;
> -        InQuotationMode = FALSE;
> -        for (Index = 0; Index < StringLen; Index++) {
> -          if (CurrentString[Index] == L'\"') {
> -            InQuotationMode = (BOOLEAN)(!InQuotationMode);
> -          }
> -          if (CurrentString[Index] == L' ' && !InQuotationMode) {
> -            TabPos = Index + 1;
> -            TabUpdatePos = Index + 1;
> -          }
> -          if (CurrentString[Index] == L'\\') {
> -            TabUpdatePos = Index + 1;
> -          }
> +      if (!InTabScrolling) {
> +        TabCurrent = NULL;
> +        //
> +        // Initialize a tab complete operation.
> +        //
> +        Status = CreateTabCompletionList (CurrentString, StringLen, *BufferSize,
> &TabCompleteList, &TabUpdatePos);
> +        if (!EFI_ERROR(Status)) {
> +          InTabScrolling = TRUE;
>          }
> -        if (StrStr(CurrentString + TabPos, L":") == NULL) {
> -          Cwd = ShellInfoObject.NewEfiShellProtocol->GetCurDir(NULL);
> -          if (Cwd != NULL) {
> -            StrnCpyS(TabStr, (*BufferSize)/sizeof(CHAR16), Cwd,
> (*BufferSize)/sizeof(CHAR16) - 1);
> -            StrCatS(TabStr, (*BufferSize)/sizeof(CHAR16), L"\\");
> -            if (TabStr[StrLen(TabStr)-1] == L'\\' && *(CurrentString + TabPos) ==
> L'\\' ) {
> -              TabStr[StrLen(TabStr)-1] = CHAR_NULL;
> -            }
> -            StrnCatS( TabStr,
> -                      (*BufferSize)/sizeof(CHAR16),
> -                      CurrentString + TabPos,
> -                      StringLen - TabPos
> -                      );
> -          } else {
> -            *TabStr = CHAR_NULL;
> -            StrnCatS(TabStr, (*BufferSize)/sizeof(CHAR16), CurrentString +
> TabPos, StringLen - TabPos);
> -          }
> +
> +        //
> +        // We do not set up the replacement.
> +        // The next section will do that.
> +        //
> +      }
> +
> +      if (InTabScrolling) {
> +        //
> +        // We are in a tab complete operation.
> +        // set up the next replacement.
> +        //
> +        ASSERT(TabCompleteList != NULL);
> +        if (TabCurrent == NULL) {
> +          TabCurrent = (EFI_SHELL_FILE_INFO*) GetFirstNode
> + (&TabCompleteList->Link);
>          } else {
> -          StrnCpyS(TabStr, (*BufferSize)/sizeof(CHAR16), CurrentString + TabPos,
> (*BufferSize)/sizeof(CHAR16) - 1);
> +          TabCurrent = (EFI_SHELL_FILE_INFO*) GetNextNode
> + (&TabCompleteList->Link, &TabCurrent->Link);
>          }
> -        StrnCatS(TabStr, (*BufferSize)/sizeof(CHAR16), L"*",
> (*BufferSize)/sizeof(CHAR16) - 1 - StrLen(TabStr));
> -        FoundFileList = NULL;
> -        Status  = ShellInfoObject.NewEfiShellProtocol->FindFiles(TabStr,
> &FoundFileList);
> -        for ( TempStr = CurrentString
> -            ; *TempStr == L' '
> -            ; TempStr++); // note the ';'... empty for loop
> +
>          //
> -        // make sure we have a list before we do anything more...
> +        // Skip over the empty list beginning node
>          //
> -        if (EFI_ERROR (Status) || FoundFileList == NULL) {
> -          InTabScrolling = FALSE;
> -          TabLinePos = NULL;
> -          continue;
> -        } else {
> -          //
> -          // enumerate through the list of files
> -          //
> -          for ( TempPos =
> (EFI_SHELL_FILE_INFO*)GetFirstNode(&(FoundFileList->Link))
> -              ; !IsNull(&FoundFileList->Link, &TempPos->Link)
> -              ; TempPos = (EFI_SHELL_FILE_INFO*)GetNextNode(&(FoundFileList-
> >Link), &(TempPos->Link))
> -             ){
> -            //
> -            // If "cd" is typed, only directory name will be auto-complete filled
> -            // in either case . and .. will be removed.
> -            //
> -            if ((((TempStr[0] == L'c' || TempStr[0] == L'C') &&
> -                (TempStr[1] == L'd' || TempStr[1] == L'D')
> -               ) && ((ShellIsDirectory(TempPos->FullName) != EFI_SUCCESS)
> -                ||(StrCmp(TempPos->FileName, L".") == 0)
> -                ||(StrCmp(TempPos->FileName, L"..") == 0)
> -               )) || ((StrCmp(TempPos->FileName, L".") == 0)
> -                ||(StrCmp(TempPos->FileName, L"..") == 0))){
> -                TabLinePos = TempPos;
> -                TempPos = (EFI_SHELL_FILE_INFO*)(RemoveEntryList(&(TempPos-
> >Link))->BackLink);
> -                InternalFreeShellFileInfoNode(TabLinePos);
> -            }
> -          }
> -          if (FoundFileList != NULL && !IsListEmpty(&FoundFileList->Link)) {
> -            TabLinePos = (EFI_SHELL_FILE_INFO*)GetFirstNode(&FoundFileList-
> >Link);
> -            InTabScrolling = TRUE;
> -          } else {
> -            ShellInfoObject.NewEfiShellProtocol->FreeFileList (&FoundFileList);
> -          }
> +        if (IsNull(&TabCompleteList->Link, &TabCurrent->Link)) {
> +          TabCurrent = (EFI_SHELL_FILE_INFO*) GetNextNode
> + (&TabCompleteList->Link, &TabCurrent->Link);
>          }
>        }
>        break;
> @@ -720,23 +770,31 @@ FileInterfaceStdInRead(
>      // the next file or directory name
>      //
>      if (InTabScrolling) {
> +      TabOutputStr = AllocateZeroPool (*BufferSize);
> +      if (TabOutputStr == NULL) {
> +        Status = EFI_OUT_OF_RESOURCES;
> +      }
> +    }
> +
> +    if (InTabScrolling && TabOutputStr != NULL) {
> +
>        //
>        // Adjust the column and row to the start of TAB-completion string.
>        //
>        Column = (StartColumn + TabUpdatePos) % TotalColumn;
>        Row -= (StartColumn + StringCurPos) / TotalColumn - (StartColumn +
> TabUpdatePos) / TotalColumn;
> -      OutputLength = StrLen (TabLinePos->FileName);
> +      OutputLength = StrLen (TabCurrent->FileName);
>        //
>        // if the output string contains  blank space, quotation marks L'\"'
>        // should be added to the output.
>        //
> -      if (StrStr(TabLinePos->FileName, L" ") != NULL){
> +      if (StrStr(TabCurrent->FileName, L" ") != NULL){
>          TabOutputStr[0] = L'\"';
> -        CopyMem (TabOutputStr + 1, TabLinePos->FileName, OutputLength *
> sizeof (CHAR16));
> +        CopyMem (TabOutputStr + 1, TabCurrent->FileName, OutputLength *
> + sizeof (CHAR16));
>          TabOutputStr[OutputLength + 1] = L'\"';
>          TabOutputStr[OutputLength + 2] = CHAR_NULL;
>        } else {
> -        CopyMem (TabOutputStr, TabLinePos->FileName, OutputLength *
> sizeof (CHAR16));
> +        CopyMem (TabOutputStr, TabCurrent->FileName, OutputLength *
> + sizeof (CHAR16));
>          TabOutputStr[OutputLength] = CHAR_NULL;
>        }
>        OutputLength = StrLen (TabOutputStr) < MaxStr - 1 ? StrLen
> (TabOutputStr) : MaxStr - 1; @@ -747,6 +805,8 @@ FileInterfaceStdInRead(
>        if (StringLen > TabUpdatePos + OutputLength) {
>          Delete = StringLen - TabUpdatePos - OutputLength;
>        }
> +
> +      FreePool(TabOutputStr);
>      }
> 
>      //
> @@ -850,8 +910,6 @@ FileInterfaceStdInRead(
>      AddLineToCommandHistory(CurrentString);
>    }
> 
> -  FreePool (TabStr);
> -  FreePool (TabOutputStr);
>    //
>    // Return the data to the caller
>    //
> @@ -861,10 +919,10 @@ FileInterfaceStdInRead(
>    // if this was used it should be deallocated by now...
>    // prevent memory leaks...
>    //
> -  if (FoundFileList != NULL) {
> -    ShellInfoObject.NewEfiShellProtocol->FreeFileList (&FoundFileList);
> +  if (TabCompleteList != NULL) {
> +    ShellInfoObject.NewEfiShellProtocol->FreeFileList
> + (&TabCompleteList);
>    }
> -  ASSERT(FoundFileList == NULL);
> +  ASSERT(TabCompleteList == NULL);
> 
>    return Status;
>  }
> --
> 2.9.0.windows.1
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel


  reply	other threads:[~2016-08-09  5:44 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-08-09  5:43 [PATCH v2 1/6] ShellPkg: TAB logic incorrectly chops out fs0: when typing fs0:<TAB> Ruiyu Ni
2016-08-09  5:44 ` Ni, Ruiyu [this message]
2016-08-09 15:08 ` 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=734D49CCEBEEF84792F5B80ED585239D1ADB4A20@SHSMSX103.ccr.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