ShellPkg: Fix ASCII and UNICODE file pipes.

Fix various errors when piping a UNICODE or ASCII file to a simple shell application that reads standard input and writes it to standard output.

1) When the memory file is created by CreateFileInferfaceMem() to capture the pipe output, no UNICODE BOM is written to the memory file. Later, when the memory file is read by the application using ShellFileHandleReadLine(), the function indicates that the file is ASCII because there is no BOM.

2) If the file is piped as ASCII, the ASCII memory image is not correctly created by FileInterfaceMemWrite() as each ASCII character is followed by '\0' in the image (when the ASCII data is written to the memory image, the file position should only be incremented by half the buffer size).

3) ShellFileHandleReadLine() does not read ASCII files correctly (writes to Buffer need to be cast as CHAR8*).

4) FileInterfaceMemRead() and FileInterfaceMemWrite() as somewhat hard to read and difficult to debug with certain tools due to the typecasting of This. Added a local variable (MemFile) of the correct type to these functions and used it instead of This.

Enhancement: ShellFileHandleReadLine() now returns EFI_END_OF_FILE when appropriate.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jim Dailey <jim_dailey@dell.com>
reviewed-by: Jaben Carsey <jaben.carsey@intel.com>

diff --git a/ShellPkg/Application/Shell/FileHandleWrappers.c b/ShellPkg/Application/Shell/FileHandleWrappers.c
index 64f382718b8509c3225361ce384fa3e06428d748..168b78bd25fa9b77537c6130ad952998c13dcece 100644 (file)
--- a/ShellPkg/Application/Shell/FileHandleWrappers.c
+++ b/ShellPkg/Application/Shell/FileHandleWrappers.c
@@ -2,6 +2,7 @@
   EFI_FILE_PROTOCOL wrappers for other items (Like Environment Variables,\r
   StdIn, StdOut, StdErr, etc...).\r
 \r
+  Copyright 2016 Dell Inc.\r
   Copyright (c) 2009 - 2015, Intel Corporation. All rights reserved.<BR>\r
   (C) Copyright 2013 Hewlett-Packard Development Company, L.P.<BR>\r
   This program and the accompanying materials\r
@@ -1385,17 +1386,20 @@ FileInterfaceMemWrite(
   IN VOID *Buffer\r
   )\r
 {\r
-  CHAR8 *AsciiBuffer;\r
-  if (((EFI_FILE_PROTOCOL_MEM*)This)->Unicode) {\r
+  CHAR8                  *AsciiBuffer;\r
+  EFI_FILE_PROTOCOL_MEM  *MemFile;\r
+\r
+  MemFile = (EFI_FILE_PROTOCOL_MEM *) This;\r
+  if (MemFile->Unicode) {\r
     //\r
     // Unicode\r
     //\r
-    if ((UINTN)(((EFI_FILE_PROTOCOL_MEM*)This)->Position + (*BufferSize)) > (UINTN)(((EFI_FILE_PROTOCOL_MEM*)This)->BufferSize)) {\r
-      ((EFI_FILE_PROTOCOL_MEM*)This)->Buffer = ReallocatePool((UINTN)(((EFI_FILE_PROTOCOL_MEM*)This)->BufferSize), (UINTN)(((EFI_FILE_PROTOCOL_MEM*)This)->BufferSize) + (*BufferSize) + 10, ((EFI_FILE_PROTOCOL_MEM*)This)->Buffer);\r
-      ((EFI_FILE_PROTOCOL_MEM*)This)->BufferSize += (*BufferSize) + 10;\r
+    if ((UINTN)(MemFile->Position + (*BufferSize)) > (UINTN)(MemFile->BufferSize)) {\r
+      MemFile->Buffer = ReallocatePool((UINTN)(MemFile->BufferSize), (UINTN)(MemFile->BufferSize) + (*BufferSize) + 10, MemFile->Buffer);\r
+      MemFile->BufferSize += (*BufferSize) + 10;\r
     }\r
-    CopyMem(((UINT8*)((EFI_FILE_PROTOCOL_MEM*)This)->Buffer) + ((EFI_FILE_PROTOCOL_MEM*)This)->Position, Buffer, *BufferSize);\r
-    ((EFI_FILE_PROTOCOL_MEM*)This)->Position += (*BufferSize);\r
+    CopyMem(((UINT8*)MemFile->Buffer) + MemFile->Position, Buffer, *BufferSize);\r
+    MemFile->Position += (*BufferSize);\r
     return (EFI_SUCCESS);\r
   } else {\r
     //\r
@@ -1406,12 +1410,12 @@ FileInterfaceMemWrite(
       return (EFI_OUT_OF_RESOURCES);\r
     }\r
     AsciiSPrint(AsciiBuffer, *BufferSize, "%S", Buffer);\r
-    if ((UINTN)(((EFI_FILE_PROTOCOL_MEM*)This)->Position + AsciiStrSize(AsciiBuffer)) > (UINTN)(((EFI_FILE_PROTOCOL_MEM*)This)->BufferSize)) {\r
-      ((EFI_FILE_PROTOCOL_MEM*)This)->Buffer = ReallocatePool((UINTN)(((EFI_FILE_PROTOCOL_MEM*)This)->BufferSize), (UINTN)(((EFI_FILE_PROTOCOL_MEM*)This)->BufferSize) + AsciiStrSize(AsciiBuffer) + 10, ((EFI_FILE_PROTOCOL_MEM*)This)->Buffer);\r
-      ((EFI_FILE_PROTOCOL_MEM*)This)->BufferSize += AsciiStrSize(AsciiBuffer) + 10;\r
+    if ((UINTN)(MemFile->Position + AsciiStrSize(AsciiBuffer)) > (UINTN)(MemFile->BufferSize)) {\r
+      MemFile->Buffer = ReallocatePool((UINTN)(MemFile->BufferSize), (UINTN)(MemFile->BufferSize) + AsciiStrSize(AsciiBuffer) + 10, MemFile->Buffer);\r
+      MemFile->BufferSize += AsciiStrSize(AsciiBuffer) + 10;\r
     }\r
-    CopyMem(((UINT8*)((EFI_FILE_PROTOCOL_MEM*)This)->Buffer) + ((EFI_FILE_PROTOCOL_MEM*)This)->Position, AsciiBuffer, AsciiStrSize(AsciiBuffer));\r
-    ((EFI_FILE_PROTOCOL_MEM*)This)->Position += AsciiStrSize(AsciiBuffer);\r
+    CopyMem(((UINT8*)MemFile->Buffer) + MemFile->Position, AsciiBuffer, AsciiStrSize(AsciiBuffer));\r
+    MemFile->Position += (*BufferSize / sizeof(CHAR16));\r
     FreePool(AsciiBuffer);\r
     return (EFI_SUCCESS);\r
   }\r
@@ -1434,11 +1438,14 @@ FileInterfaceMemRead(
   IN VOID *Buffer\r
   )\r
 {\r
-  if (*BufferSize > (UINTN)((((EFI_FILE_PROTOCOL_MEM*)This)->BufferSize) - (UINTN)(((EFI_FILE_PROTOCOL_MEM*)This)->Position))) {\r
-    (*BufferSize) = (UINTN)((((EFI_FILE_PROTOCOL_MEM*)This)->BufferSize) - (UINTN)(((EFI_FILE_PROTOCOL_MEM*)This)->Position));\r
+  EFI_FILE_PROTOCOL_MEM  *MemFile;\r
+\r
+  MemFile = (EFI_FILE_PROTOCOL_MEM *) This;\r
+  if (*BufferSize > (UINTN)((MemFile->BufferSize) - (UINTN)(MemFile->Position))) {\r
+    (*BufferSize) = (UINTN)((MemFile->BufferSize) - (UINTN)(MemFile->Position));\r
   }\r
-  CopyMem(Buffer, ((UINT8*)((EFI_FILE_PROTOCOL_MEM*)This)->Buffer) + ((EFI_FILE_PROTOCOL_MEM*)This)->Position, (*BufferSize));\r
-  ((EFI_FILE_PROTOCOL_MEM*)This)->Position = ((EFI_FILE_PROTOCOL_MEM*)This)->Position + (*BufferSize);\r
+  CopyMem(Buffer, ((UINT8*)MemFile->Buffer) + MemFile->Position, (*BufferSize));\r
+  MemFile->Position = MemFile->Position + (*BufferSize);\r
   return (EFI_SUCCESS);\r
 }\r
 \r
@@ -1507,6 +1514,13 @@ CreateFileInterfaceMem(
   ASSERT(FileInterface->BufferSize  == 0);\r
   ASSERT(FileInterface->Position    == 0);\r
 \r
+  if (Unicode) {\r
+    FileInterface->Buffer = AllocateZeroPool(sizeof(gUnicodeFileTag));\r
+    *((CHAR16 *) (FileInterface->Buffer)) = EFI_UNICODE_BYTE_ORDER_MARK;\r
+    FileInterface->BufferSize = 2;\r
+    FileInterface->Position = 2;\r
+  }\r
+\r
   return ((EFI_FILE_PROTOCOL *)FileInterface);\r
 }\r
 \r

diff --git a/ShellPkg/Include/Library/ShellLib.h b/ShellPkg/Include/Library/ShellLib.h
index d02d6e9de1f162907b344201fc508dbebf3a033b..88020a06a89c57cc3540ee0a7a97dbc184b87e56 100644 (file)
--- a/ShellPkg/Include/Library/ShellLib.h
+++ b/ShellPkg/Include/Library/ShellLib.h
@@ -1333,6 +1333,7 @@ ShellFileHandleReturnLine(
 \r
   @retval EFI_SUCCESS           The operation was successful.  The line is stored in\r
                                 Buffer.\r
+  @retval EFI_END_OF_FILE       There are no more lines in the file.\r
   @retval EFI_INVALID_PARAMETER Handle was NULL.\r
   @retval EFI_INVALID_PARAMETER Size was NULL.\r
   @retval EFI_BUFFER_TOO_SMALL  Size was not large enough to store the line.\r

diff --git a/ShellPkg/Library/UefiShellLib/UefiShellLib.c b/ShellPkg/Library/UefiShellLib/UefiShellLib.c
index 9e815c56a6c8f7106965c04de1c90397ba465c7f..4b53c7080c306f9e69335f2b4a0c8c63b2aaa060 100644 (file)
--- a/ShellPkg/Library/UefiShellLib/UefiShellLib.c
+++ b/ShellPkg/Library/UefiShellLib/UefiShellLib.c
@@ -1,6 +1,7 @@
 /** @file\r
   Provides interface to shell functionality for shell commands and applications.\r
 \r
+  Copyright 2016 Dell Inc.\r
   Copyright (c) 2006 - 2015, Intel Corporation. All rights reserved.<BR>\r
   This program and the accompanying materials\r
   are licensed and made available under the terms and conditions of the BSD License\r
@@ -4097,6 +4098,7 @@ ShellFileHandleReturnLine(
 \r
   @retval EFI_SUCCESS           The operation was successful.  The line is stored in\r
                                 Buffer.\r
+  @retval EFI_END_OF_FILE       There are no more lines in the file.\r
   @retval EFI_INVALID_PARAMETER Handle was NULL.\r
   @retval EFI_INVALID_PARAMETER Size was NULL.\r
   @retval EFI_BUFFER_TOO_SMALL  Size was not large enough to store the line.\r
@@ -4142,45 +4144,64 @@ ShellFileHandleReadLine(
     }\r
   }\r
 \r
+  if (*Ascii) {\r
+    CharSize = sizeof(CHAR8);\r
+  } else {\r
+    CharSize = sizeof(CHAR16);\r
+  }\r
   for (CountSoFar = 0;;CountSoFar++){\r
     CharBuffer = 0;\r
-    if (*Ascii) {\r
-      CharSize = sizeof(CHAR8);\r
-    } else {\r
-      CharSize = sizeof(CHAR16);\r
-    }\r
     Status = gEfiShellProtocol->ReadFile(Handle, &CharSize, &CharBuffer);\r
     if (  EFI_ERROR(Status)\r
        || CharSize == 0\r
        || (CharBuffer == L'\n' && !(*Ascii))\r
        || (CharBuffer ==  '\n' && *Ascii)\r
      ){\r
+      if (CharSize == 0) {\r
+        Status = EFI_END_OF_FILE;\r
+      }\r
       break;\r
     }\r
     //\r
     // if we have space save it...\r
     //\r
-    if ((CountSoFar+1)*sizeof(CHAR16) < *Size){\r
+    if ((CountSoFar + 1) * CharSize < *Size){\r
       ASSERT(Buffer != NULL);\r
-      ((CHAR16*)Buffer)[CountSoFar] = CharBuffer;\r
-      ((CHAR16*)Buffer)[CountSoFar+1] = CHAR_NULL;\r
+      if (*Ascii) {\r
+        ((CHAR8*)Buffer)[CountSoFar] = (CHAR8) CharBuffer;\r
+        ((CHAR8*)Buffer)[CountSoFar+1] = '\0';\r
+      }\r
+      else {\r
+        ((CHAR16*)Buffer)[CountSoFar] = CharBuffer;\r
+        ((CHAR16*)Buffer)[CountSoFar+1] = CHAR_NULL;\r
+      }\r
     }\r
   }\r
 \r
   //\r
   // if we ran out of space tell when...\r
   //\r
-  if ((CountSoFar+1)*sizeof(CHAR16) > *Size){\r
-    *Size = (CountSoFar+1)*sizeof(CHAR16);\r
-    if (!Truncate) {\r
-      gEfiShellProtocol->SetFilePosition(Handle, OriginalFilePosition);\r
-    } else {\r
-      DEBUG((DEBUG_WARN, "The line was truncated in ShellFileHandleReadLine"));\r
+  if (Status != EFI_END_OF_FILE){\r
+    if ((CountSoFar + 1) * CharSize > *Size){\r
+      *Size = (CountSoFar + 1) * CharSize;\r
+      if (!Truncate) {\r
+        gEfiShellProtocol->SetFilePosition(Handle, OriginalFilePosition);\r
+      } else {\r
+        DEBUG((DEBUG_WARN, "The line was truncated in ShellFileHandleReadLine"));\r
+      }\r
+      return (EFI_BUFFER_TOO_SMALL);\r
+    }\r
+\r
+    if (*Ascii) {\r
+      if (CountSoFar && ((CHAR8*)Buffer)[CountSoFar - 1] == '\r') {\r
+        ((CHAR8*)Buffer)[CountSoFar - 1] = '\0';\r
+      }\r
+    }\r
+    else {\r
+      if (CountSoFar && Buffer[CountSoFar - 1] == L'\r') {\r
+        Buffer[CountSoFar - 1] = CHAR_NULL;\r
+      }\r
     }\r
-    return (EFI_BUFFER_TOO_SMALL);\r
-  }\r
-  while(Buffer[StrLen(Buffer)-1] == L'\r') {\r
-    Buffer[StrLen(Buffer)-1] = CHAR_NULL;\r
   }\r
 \r
   return (Status);\r

diff --git a/ShellPkg/Library/UefiShellLib/UefiShellLib.inf b/ShellPkg/Library/UefiShellLib/UefiShellLib.inf
index 6ed76cc2bc881c4b4e0c1226ba954b6d503886db..d44ce02bb38ec1069d56579fb5bc7e9cbd68205d 100644 (file)
--- a/ShellPkg/Library/UefiShellLib/UefiShellLib.inf
+++ b/ShellPkg/Library/UefiShellLib/UefiShellLib.inf
@@ -18,7 +18,7 @@
   BASE_NAME                      = UefiShellLib\r
   FILE_GUID                      = 449D0F00-2148-4a43-9836-F10B3980ECF5\r
   MODULE_TYPE                    = UEFI_DRIVER\r
-  VERSION_STRING                 = 1.0\r
+  VERSION_STRING                 = 1.1\r
   LIBRARY_CLASS                  = ShellLib|UEFI_APPLICATION UEFI_DRIVER DXE_RUNTIME_DRIVER DXE_DRIVER\r
   CONSTRUCTOR                    = ShellLibConstructor\r
   DESTRUCTOR                     = ShellLibDestructor\r