[PATCH v2 0/9] EmbeddedPkg: eliminate calls to deprecated functions

[PATCH v2 0/9] EmbeddedPkg: eliminate calls to deprecated functions

[Ard Biesheuvel]

This series prepares EmbeddedPkg for building with the preprocess symbol
DISABLE_NEW_DEPRECATED_INTERFACES defined, by adding missing components
to EmbeddedPkg (#3), fixing broken code or code that relies on deprecated
functionality (#1 - #2, #4 - #8), and finally adds
-DDISABLE_NEW_DEPRECATED_INTERFACES to the CC flags for all build types,
toolchains and architectures.

v2:
- incorporated Laszlo's feedback
- add some R-b's

Again, a *big* thank you to Laszlo for taking the time to review these
patches. I do apologize for not being as thorough as I could have been,
resulting in issues in the code that Laszlo spotted in his review.

Bug: https://bugzilla.tianocore.org/show_bug.cgi?id=164

Ard Biesheuvel (9):
  EmbeddedPkg/AndroidFastbootTransportTcpDxe: remove broken hostname
    handling
  EmbeddedPkg: remove unused PrePiHobListPointerLib
  EmbeddedPkg: add missing modules
  EmbeddedPkg/GdbDebugAgent: fix VOID* cast of incorrect size
  EmbeddedPkg/AndroidFastboot: eliminate deprecated string function
    calls
  EmbeddedPkg/Ebl: eliminate deprecated string function calls
  EmbeddedPkg/EfiFileLib: eliminate deprecated string function calls
  EmbeddedPkg/MmcDxe: eliminate deprecated string function calls
  EmbeddedPkg: enable -DDISABLE_NEW_DEPRECATED_INTERFACES

 EmbeddedPkg/Application/AndroidFastboot/AndroidBootImg.c                       |  3 +-
 EmbeddedPkg/Application/AndroidFastboot/AndroidFastbootApp.c                   | 11 ++--
 EmbeddedPkg/Drivers/AndroidFastbootTransportTcpDxe/FastbootTransportTcp.c      | 23 -------
 EmbeddedPkg/Drivers/AndroidFastbootTransportTcpDxe/FastbootTransportTcpDxe.inf |  3 -
 EmbeddedPkg/Ebl/Command.c                                                      |  2 +-
 EmbeddedPkg/Ebl/Dir.c                                                          |  4 +-
 EmbeddedPkg/Ebl/EfiDevice.c                                                    | 11 ++--
 EmbeddedPkg/Ebl/Main.c                                                         |  8 +--
 EmbeddedPkg/Ebl/Variable.c                                                     | 17 +++--
 EmbeddedPkg/EmbeddedPkg.dec                                                    |  1 -
 EmbeddedPkg/EmbeddedPkg.dsc                                                    | 13 +++-
 EmbeddedPkg/Library/EfiFileLib/EfiFileLib.c                                    | 42 ++++++------
 EmbeddedPkg/Library/GdbDebugAgent/Arm/Processor.c                              |  2 +-
 EmbeddedPkg/Library/PrePiHobListPointerLib/PrePiHobListPointer.c               | 69 --------------------
 EmbeddedPkg/Library/PrePiHobListPointerLib/PrePiHobListPointerLib.inf          | 38 -----------
 EmbeddedPkg/Universal/MmcDxe/Diagnostics.c                                     |  4 +-
 Omap35xxPkg/Omap35xxPkg.dsc                                                    |  1 -
 17 files changed, 69 insertions(+), 183 deletions(-)
 delete mode 100644 EmbeddedPkg/Library/PrePiHobListPointerLib/PrePiHobListPointer.c
 delete mode 100644 EmbeddedPkg/Library/PrePiHobListPointerLib/PrePiHobListPointerLib.inf

-- 
2.7.4

[PATCH v2 1/9] EmbeddedPkg/AndroidFastbootTransportTcpDxe: remove broken hostname handling

[Ard Biesheuvel]

The fastboot TCP connection setup routine retrieves a hostname from a
UEFI variable 'hostname' that is scoped under a GUID gEfiHostnameVariableGuid
whose definition is missing from the code. Since the hostname is only printed
and then discarded, let's just drop the whole thing.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 EmbeddedPkg/Drivers/AndroidFastbootTransportTcpDxe/FastbootTransportTcp.c      | 23 --------------------
 EmbeddedPkg/Drivers/AndroidFastbootTransportTcpDxe/FastbootTransportTcpDxe.inf |  3 ---
 2 files changed, 26 deletions(-)

diff --git a/EmbeddedPkg/Drivers/AndroidFastbootTransportTcpDxe/FastbootTransportTcp.c b/EmbeddedPkg/Drivers/AndroidFastbootTransportTcpDxe/FastbootTransportTcp.c
index 8ec78aeb534e..7c008ac9722e 100644
--- a/EmbeddedPkg/Drivers/AndroidFastbootTransportTcpDxe/FastbootTransportTcp.c
+++ b/EmbeddedPkg/Drivers/AndroidFastbootTransportTcpDxe/FastbootTransportTcp.c
@@ -27,8 +27,6 @@
 #include <Library/UefiDriverEntryPoint.h>
 #include <Library/UefiRuntimeServicesTableLib.h>
 
-#include <Guid/Hostname.h>
-
 #define IP4_ADDR_TO_STRING(IpAddr, IpAddrString) UnicodeSPrint (       \
                                                    IpAddrString,       \
                                                    16 * 2,             \
@@ -307,9 +305,6 @@ TcpFastbootTransportStart (
   EFI_HANDLE                   *HandleBuffer;
   EFI_IP4_MODE_DATA             Ip4ModeData;
   UINTN                         NumHandles;
-  UINTN                         HostnameSize = 256;
-  CHAR8                         Hostname[256];
-  CHAR16                        HostnameUnicode[256] = L"<no hostname>";
   CHAR16                        IpAddrString[16];
   UINTN                         Index;
 
@@ -442,28 +437,10 @@ TcpFastbootTransportStart (
   //
   IP4_ADDR_TO_STRING (Ip4ModeData.ConfigData.StationAddress, IpAddrString);
 
-  // Look up hostname
-  Status = gRT->GetVariable (
-                  L"Hostname",
-                  &gEfiHostnameVariableGuid,
-                  NULL,
-                  &HostnameSize,
-                  &Hostname
-                  );
-  if (!EFI_ERROR (Status) && HostnameSize != 0) {
-    AsciiStrToUnicodeStr (Hostname, HostnameUnicode);
-  }
-
-  // Hostname variable is not null-terminated.
-  Hostname[HostnameSize] = L'\0';
-
   mTextOut->OutputString (mTextOut, L"TCP Fastboot transport configured.");
   mTextOut->OutputString (mTextOut, L"\r\nIP address: ");
   mTextOut->OutputString (mTextOut ,IpAddrString);
   mTextOut->OutputString (mTextOut, L"\r\n");
-  mTextOut->OutputString (mTextOut, L"\r\nhostname: ");
-  mTextOut->OutputString (mTextOut, HostnameUnicode);
-  mTextOut->OutputString (mTextOut, L"\r\n");
 
   //
   // Start listening for a connection
diff --git a/EmbeddedPkg/Drivers/AndroidFastbootTransportTcpDxe/FastbootTransportTcpDxe.inf b/EmbeddedPkg/Drivers/AndroidFastbootTransportTcpDxe/FastbootTransportTcpDxe.inf
index 4d777934e5a4..89ff556b40ca 100644
--- a/EmbeddedPkg/Drivers/AndroidFastbootTransportTcpDxe/FastbootTransportTcpDxe.inf
+++ b/EmbeddedPkg/Drivers/AndroidFastbootTransportTcpDxe/FastbootTransportTcpDxe.inf
@@ -47,8 +47,5 @@ [Packages]
   MdeModulePkg/MdeModulePkg.dec
   EmbeddedPkg/EmbeddedPkg.dec
 
-[Guids]
-  gEfiHostnameVariableGuid
-
 [FixedPcd]
   gEmbeddedTokenSpaceGuid.PcdAndroidFastbootTcpPort
-- 
2.7.4

[PATCH v2 2/9] EmbeddedPkg: remove unused PrePiHobListPointerLib

[Ard Biesheuvel]

Remove this unused version: all existing platforms use the one under
ArmPlatformPkg instead.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 EmbeddedPkg/EmbeddedPkg.dec                                           |  1 -
 EmbeddedPkg/EmbeddedPkg.dsc                                           |  1 -
 EmbeddedPkg/Library/PrePiHobListPointerLib/PrePiHobListPointer.c      | 69 --------------------
 EmbeddedPkg/Library/PrePiHobListPointerLib/PrePiHobListPointerLib.inf | 38 -----------
 Omap35xxPkg/Omap35xxPkg.dsc                                           |  1 -
 5 files changed, 110 deletions(-)

diff --git a/EmbeddedPkg/EmbeddedPkg.dec b/EmbeddedPkg/EmbeddedPkg.dec
index 775d863c5f64..2c2cf41103c2 100644
--- a/EmbeddedPkg/EmbeddedPkg.dec
+++ b/EmbeddedPkg/EmbeddedPkg.dec
@@ -97,7 +97,6 @@ [PcdsFixedAtBuild.common]
   gEmbeddedTokenSpaceGuid.PcdEmbeddedMemVariableStoreSize|0x10000|UINT32|0x00000009
   gEmbeddedTokenSpaceGuid.PcdEmbeddedPrompt|"Ebl"|VOID*|0x00000034
 
-  gEmbeddedTokenSpaceGuid.PcdPrePiHobBase|131072|UINT32|0x00000040
   gEmbeddedTokenSpaceGuid.PcdPrePiStackBase|0|UINT32|0x0000000b
   gEmbeddedTokenSpaceGuid.PcdPrePiStackSize|131072|UINT32|0x0000000c
 
diff --git a/EmbeddedPkg/EmbeddedPkg.dsc b/EmbeddedPkg/EmbeddedPkg.dsc
index cd1f6df06b24..5224bcefa9c7 100644
--- a/EmbeddedPkg/EmbeddedPkg.dsc
+++ b/EmbeddedPkg/EmbeddedPkg.dsc
@@ -184,7 +184,6 @@ [PcdsFixedAtBuild.common]
   gEmbeddedTokenSpaceGuid.PcdEmbeddedDefaultTextColor|0x07
   gEmbeddedTokenSpaceGuid.PcdEmbeddedMemVariableStoreSize|0x10000
 
-  gEmbeddedTokenSpaceGuid.PcdPrePiHobBase|0
   gEmbeddedTokenSpaceGuid.PcdPrePiStackBase|0
   gEmbeddedTokenSpaceGuid.PcdPrePiStackSize|0
 
diff --git a/EmbeddedPkg/Library/PrePiHobListPointerLib/PrePiHobListPointer.c b/EmbeddedPkg/Library/PrePiHobListPointerLib/PrePiHobListPointer.c
deleted file mode 100644
index 9065b1132fda..000000000000
--- a/EmbeddedPkg/Library/PrePiHobListPointerLib/PrePiHobListPointer.c
+++ /dev/null
@@ -1,69 +0,0 @@
-/** @file
-*
-*  Copyright (c) 2011, ARM Limited. All rights reserved.
-*
-*  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
-*  http://opensource.org/licenses/bsd-license.php
-*
-*  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
-*  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
-*
-**/
-
-#include <PiPei.h>
-#include <Library/PrePiHobListPointerLib.h>
-#include <Library/DebugLib.h>
-#include <Library/PcdLib.h>
-
-//
-// Have to use build system to set the original value in case we are running
-// from FLASH and globals don't work. So if you do a GetHobList() and gHobList
-// and gHobList is NULL the PCD default values are used.
-//
-VOID *gHobList = NULL;
-
-
-/**
-  Returns the pointer to the HOB list.
-
-  This function returns the pointer to first HOB in the list.
-
-  @return The pointer to the HOB list.
-
-**/
-VOID *
-EFIAPI
-PrePeiGetHobList (
-  VOID
-  )
-{
-  if (gHobList == NULL) {
-    return (VOID *)*(UINTN*)PcdGet32 (PcdPrePiHobBase);
-  } else {
-    return gHobList;
-  }
-}
-
-
-
-/**
-  Updates the pointer to the HOB list.
-
-  @param  HobList       Hob list pointer to store
-
-**/
-EFI_STATUS
-EFIAPI
-PrePeiSetHobList (
-  IN  VOID      *HobList
-  )
-{
-  gHobList = HobList;
-
-  //
-  // If this code is running from ROM this could fail
-  //
-  return (gHobList == HobList) ? EFI_SUCCESS: EFI_UNSUPPORTED;
-}
diff --git a/EmbeddedPkg/Library/PrePiHobListPointerLib/PrePiHobListPointerLib.inf b/EmbeddedPkg/Library/PrePiHobListPointerLib/PrePiHobListPointerLib.inf
deleted file mode 100644
index bfd3c209a13b..000000000000
--- a/EmbeddedPkg/Library/PrePiHobListPointerLib/PrePiHobListPointerLib.inf
+++ /dev/null
@@ -1,38 +0,0 @@
-#/** @file
-#
-#  Copyright (c) 2011, ARM Limited. All rights reserved.
-#
-#  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
-#  http://opensource.org/licenses/bsd-license.php
-#
-#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
-#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
-#
-#**/
-
-[Defines]
-  INF_VERSION                    = 0x00010005
-  BASE_NAME                      = PrePiHobListPointerLib
-  FILE_GUID                      = 7a6a7fc0-5ee0-11e0-b47f-0002a5d5c51b
-  MODULE_TYPE                    = BASE
-  VERSION_STRING                 = 1.0
-  LIBRARY_CLASS                  = PrePiHobListPointerLib
-
-#
-#  VALID_ARCHITECTURES           = IA32 X64 IPF EBC (EBC is for build only)
-#
-
-[Sources]
-  PrePiHobListPointer.c
-
-[Packages]
-  MdePkg/MdePkg.dec
-  EmbeddedPkg/EmbeddedPkg.dec
-
-[LibraryClasses]
-  #DebugLib
-
-[FixedPcd.common]
-  gEmbeddedTokenSpaceGuid.PcdPrePiHobBase
diff --git a/Omap35xxPkg/Omap35xxPkg.dsc b/Omap35xxPkg/Omap35xxPkg.dsc
index 580be4d6340c..4a3317f255f6 100644
--- a/Omap35xxPkg/Omap35xxPkg.dsc
+++ b/Omap35xxPkg/Omap35xxPkg.dsc
@@ -141,7 +141,6 @@ [PcdsFixedAtBuild.common]
 
   gEmbeddedTokenSpaceGuid.PcdFlashFvMainBase|0
   gEmbeddedTokenSpaceGuid.PcdFlashFvMainSize|0
-  gEmbeddedTokenSpaceGuid.PcdPrePiHobBase|0x80001000
   gEmbeddedTokenSpaceGuid.PcdPrePiStackBase|0x87FE0000 # stack at top of memory
   gEmbeddedTokenSpaceGuid.PcdPrePiStackSize|0x20000  # 128K stack
   gArmTokenSpaceGuid.PcdCpuVectorBaseAddress|0x80000000
-- 
2.7.4

[PATCH v2 3/9] EmbeddedPkg: add missing modules

[Ard Biesheuvel]

Add modules that live under EmbeddedPkg but were missing from the
[Components] section of the package .dsc file

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 EmbeddedPkg/EmbeddedPkg.dsc | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/EmbeddedPkg/EmbeddedPkg.dsc b/EmbeddedPkg/EmbeddedPkg.dsc
index 5224bcefa9c7..cc95564f31af 100644
--- a/EmbeddedPkg/EmbeddedPkg.dsc
+++ b/EmbeddedPkg/EmbeddedPkg.dsc
@@ -280,5 +280,14 @@ [Components.common]
   EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118Dxe.inf
   EmbeddedPkg/Drivers/SataSiI3132Dxe/SataSiI3132Dxe.inf
 
+  EmbeddedPkg/Library/AcpiLib/AcpiLib.inf
+  EmbeddedPkg/Library/DebugAgentTimerLibNull/DebugAgentTimerLibNull.inf
+  EmbeddedPkg/Library/DxeHobPeCoffLib/DxeHobPeCoffLib.inf
+  EmbeddedPkg/Library/EblNetworkLib/EblNetworkLib.inf
+  EmbeddedPkg/Library/FdtLib/FdtLib.inf
+  EmbeddedPkg/Library/GdbDebugAgent/GdbDebugAgent.inf
+  EmbeddedPkg/Library/PrePiHobLib/PrePiHobLib.inf
+  EmbeddedPkg/Library/PrePiMemoryAllocationLib/PrePiMemoryAllocationLib.inf
+
 [Components.IA32, Components.X64, Components.IPF, Components.ARM]
   EmbeddedPkg/GdbStub/GdbStub.inf
-- 
2.7.4

[PATCH v2 4/9] EmbeddedPkg/GdbDebugAgent: fix VOID* cast of incorrect size

[Ard Biesheuvel]

The result of PcdGet64() can only be cast to VOID* on 64-bit platforms,
so add an intermediate UINTN cast to make this code build again on 32 bit.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
---
 EmbeddedPkg/Library/GdbDebugAgent/Arm/Processor.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/EmbeddedPkg/Library/GdbDebugAgent/Arm/Processor.c b/EmbeddedPkg/Library/GdbDebugAgent/Arm/Processor.c
index 8226c80ab466..a43ff95d03a3 100644
--- a/EmbeddedPkg/Library/GdbDebugAgent/Arm/Processor.c
+++ b/EmbeddedPkg/Library/GdbDebugAgent/Arm/Processor.c
@@ -657,7 +657,7 @@ InitializeDebugAgent (
   *(UINTN *) (((UINT8 *)VectorBase) + Offset) = (UINTN)AsmCommonExceptionEntry;
 
   // Flush Caches since we updated executable stuff
-  InvalidateInstructionCacheRange ((VOID *)PcdGet64(PcdCpuVectorBaseAddress), Length);
+  InvalidateInstructionCacheRange ((VOID *)(UINTN)PcdGet64(PcdCpuVectorBaseAddress), Length);
 
   // setup a timer so gdb can break in via ctrl-c
   DebugAgentTimerIntialize ();
-- 
2.7.4

[PATCH v2 5/9] EmbeddedPkg/AndroidFastboot: eliminate deprecated string function calls

[Ard Biesheuvel]

Get rid of calls to unsafe string functions. These are deprecated and may
be removed in the future.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 EmbeddedPkg/Application/AndroidFastboot/AndroidBootImg.c     |  3 ++-
 EmbeddedPkg/Application/AndroidFastboot/AndroidFastbootApp.c | 11 ++++++-----
 2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/EmbeddedPkg/Application/AndroidFastboot/AndroidBootImg.c b/EmbeddedPkg/Application/AndroidFastboot/AndroidBootImg.c
index bbca90fc08a2..f3e770bcc980 100644
--- a/EmbeddedPkg/Application/AndroidFastboot/AndroidBootImg.c
+++ b/EmbeddedPkg/Application/AndroidFastboot/AndroidBootImg.c
@@ -84,7 +84,8 @@ ParseAndroidBootImg (
                  + ALIGN_VALUE (Header->KernelSize, Header->PageSize));
   }
 
-  AsciiStrnCpy (KernelArgs, Header->KernelArgs, BOOTIMG_KERNEL_ARGS_SIZE);
+  AsciiStrnCpyS (KernelArgs, BOOTIMG_KERNEL_ARGS_SIZE, Header->KernelArgs,
+    BOOTIMG_KERNEL_ARGS_SIZE);
 
   return EFI_SUCCESS;
 }
diff --git a/EmbeddedPkg/Application/AndroidFastboot/AndroidFastbootApp.c b/EmbeddedPkg/Application/AndroidFastboot/AndroidFastbootApp.c
index 9ddc34f57cf4..c5e8a7e34af2 100644
--- a/EmbeddedPkg/Application/AndroidFastboot/AndroidFastbootApp.c
+++ b/EmbeddedPkg/Application/AndroidFastboot/AndroidFastbootApp.c
@@ -99,7 +99,7 @@ HandleDownload (
   IN CHAR8 *NumBytesString
   )
 {
-  CHAR8       Response[12] = "DATA";
+  CHAR8       Response[13];
   CHAR16      OutputString[FASTBOOT_STRING_MAX_LENGTH];
 
   // Argument is 8-character ASCII string hex representation of number of bytes
@@ -127,8 +127,10 @@ HandleDownload (
   if (mDataBuffer == NULL) {
     SEND_LITERAL ("FAILNot enough memory");
   } else {
-    AsciiStrnCpy (Response + 4, NumBytesString, 8);
-    mTransport->Send (sizeof(Response), Response, &mFatalSendErrorEvent);
+    ZeroMem (Response, sizeof Response);
+    AsciiSPrint (Response, sizeof Response, "DATA%x",
+      (UINT32)mNumDataBytes);
+    mTransport->Send (sizeof Response - 1, Response, &mFatalSendErrorEvent);
 
     mState = ExpectDataState;
     mBytesReceivedSoFar = 0;
@@ -257,8 +259,7 @@ AcceptCmd (
   }
 
   // Commands aren't null-terminated. Let's get a null-terminated version.
-  AsciiStrnCpy (Command, Data, Size);
-  Command[Size] = '\0';
+  AsciiStrnCpyS (Command, sizeof Command, Data, Size);
 
   // Parse command
   if (MATCH_CMD_LITERAL ("getvar", Command)) {
-- 
2.7.4

[PATCH v2 6/9] EmbeddedPkg/Ebl: eliminate deprecated string function calls

[Ard Biesheuvel]

Get rid of calls to unsafe string functions. These are deprecated and may
be removed in the future.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 EmbeddedPkg/Ebl/Command.c   |  2 +-
 EmbeddedPkg/Ebl/Dir.c       |  4 ++--
 EmbeddedPkg/Ebl/EfiDevice.c | 11 ++++++-----
 EmbeddedPkg/Ebl/Main.c      |  8 ++++----
 EmbeddedPkg/Ebl/Variable.c  | 17 +++++++++++------
 5 files changed, 24 insertions(+), 18 deletions(-)

diff --git a/EmbeddedPkg/Ebl/Command.c b/EmbeddedPkg/Ebl/Command.c
index e75c6a2e5c32..4bc1f4df0ca0 100644
--- a/EmbeddedPkg/Ebl/Command.c
+++ b/EmbeddedPkg/Ebl/Command.c
@@ -614,7 +614,7 @@ OutputData (
   UINTN Spaces   = 0;
   CHAR8 Blanks[80];
 
-  AsciiStrCpy (Blanks, mBlanks);
+  AsciiStrCpyS (Blanks, sizeof Blanks, mBlanks);
   for (EndAddress = Address + Length; Address < EndAddress; Offset += Line) {
     AsciiPrint ("%08x: ", Offset);
     for (Line = 0; (Line < 0x10) && (Address < EndAddress);) {
diff --git a/EmbeddedPkg/Ebl/Dir.c b/EmbeddedPkg/Ebl/Dir.c
index 36095b633019..8dd9d48ff6ac 100644
--- a/EmbeddedPkg/Ebl/Dir.c
+++ b/EmbeddedPkg/Ebl/Dir.c
@@ -116,7 +116,7 @@ EblDirCmd (
     UnicodeFileName[0] = '\0';
     MatchSubString = &UnicodeFileName[0];
     if (Argc > 2) {
-      AsciiStrToUnicodeStr (Argv[2], UnicodeFileName);
+      AsciiStrToUnicodeStrS (Argv[2], UnicodeFileName, MAX_CMD_LINE);
       if (UnicodeFileName[0] == '*') {
         // Handle *Name substring matching
         MatchSubString = &UnicodeFileName[1];
@@ -231,7 +231,7 @@ EblDirCmd (
     MatchSubString = NULL;
     UnicodeFileName[0] = '\0';
     if (Argc > 2) {
-      AsciiStrToUnicodeStr (Argv[2], UnicodeFileName);
+      AsciiStrToUnicodeStrS (Argv[2], UnicodeFileName, MAX_CMD_LINE);
       if (UnicodeFileName[0] == '*') {
         MatchSubString = &UnicodeFileName[1];
       }
diff --git a/EmbeddedPkg/Ebl/EfiDevice.c b/EmbeddedPkg/Ebl/EfiDevice.c
index ec9c331b7004..f6969e7b2b05 100644
--- a/EmbeddedPkg/Ebl/EfiDevice.c
+++ b/EmbeddedPkg/Ebl/EfiDevice.c
@@ -343,7 +343,7 @@ EblStartCmd (
 
       ImageInfo->LoadOptionsSize = (UINT32)AsciiStrSize (Argv[2]);
       ImageInfo->LoadOptions     = AllocatePool (ImageInfo->LoadOptionsSize);
-      AsciiStrCpy (ImageInfo->LoadOptions, Argv[2]);
+      AsciiStrCpyS (ImageInfo->LoadOptions, ImageInfo->LoadOptionsSize, Argv[2]);
     }
 
     // Transfer control to the EFI image we loaded with LoadImage()
@@ -741,7 +741,7 @@ EblFileCopyCmd (
   UINTN         Size;
   UINTN         Offset;
   UINTN         Chunk        = FILE_COPY_CHUNK;
-  UINTN         FileNameLen;
+  UINTN         FileNameLen, DestFileNameLen;
   CHAR8*        DestFileName;
   CHAR8*        SrcFileName;
   CHAR8*        SrcPtr;
@@ -786,9 +786,10 @@ EblFileCopyCmd (
     }
 
     // Construct the destination filepath
-    DestFileName = (CHAR8*)AllocatePool (FileNameLen + AsciiStrLen (SrcFileName) + 1);
-    AsciiStrCpy (DestFileName, Argv[2]);
-    AsciiStrCat (DestFileName, SrcFileName);
+    DestFileNameLen = FileNameLen + AsciiStrLen (SrcFileName) + 1;
+    DestFileName = (CHAR8*)AllocatePool (DestFileNameLen);
+    AsciiStrCpyS (DestFileName, DestFileNameLen, Argv[2]);
+    AsciiStrCatS (DestFileName, DestFileNameLen, SrcFileName);
   }
 
   Source = EfiOpen(Argv[1], EFI_FILE_MODE_READ, 0);
diff --git a/EmbeddedPkg/Ebl/Main.c b/EmbeddedPkg/Ebl/Main.c
index 18b2878f69a1..62f559fccfe8 100644
--- a/EmbeddedPkg/Ebl/Main.c
+++ b/EmbeddedPkg/Ebl/Main.c
@@ -88,7 +88,7 @@ SetCmdHistory (
     }
 
     // Copy the new command line into the ring buffer
-    AsciiStrnCpy(&mCmdHistory[mCmdHistoryStart][0], Cmd, MAX_CMD_LINE);
+    AsciiStrnCpyS (&mCmdHistory[mCmdHistoryStart][0], MAX_CMD_LINE, Cmd, MAX_CMD_LINE);
   }
 
   // Reset the command history for the next up arrow press
@@ -432,7 +432,7 @@ GetCmd (
       }
       AsciiPrint (History);
       Index = AsciiStrLen (History);
-      AsciiStrnCpy (Cmd, History, CmdMaxSize);
+      AsciiStrnCpyS (Cmd, CmdMaxSize, History, CmdMaxSize);
     } else {
       Cmd[Index++] = Char;
       if (FixedPcdGetBool(PcdEmbeddedShellCharacterEcho) == TRUE) {
@@ -644,14 +644,14 @@ EdkBootLoaderEntry (
 
     Status = gRT->GetVariable(CommandLineVariableName, &VendorGuid, NULL, &CommandLineVariableSize, CommandLineVariable);
     if (!EFI_ERROR(Status)) {
-      UnicodeStrToAsciiStr(CommandLineVariable, CmdLine);
+      UnicodeStrToAsciiStrS (CommandLineVariable, CmdLine, MAX_CMD_LINE);
     }
 
     FreePool(CommandLineVariable);
   }
 
   if (EFI_ERROR(Status)) {
-    AsciiStrCpy (CmdLine, (CHAR8 *)PcdGetPtr (PcdEmbeddedAutomaticBootCommand));
+    AsciiStrCpyS (CmdLine, MAX_CMD_LINE, (CHAR8 *)PcdGetPtr (PcdEmbeddedAutomaticBootCommand));
   }
 
   for (;;) {
diff --git a/EmbeddedPkg/Ebl/Variable.c b/EmbeddedPkg/Ebl/Variable.c
index f440c48f16dd..92464a6b7133 100644
--- a/EmbeddedPkg/Ebl/Variable.c
+++ b/EmbeddedPkg/Ebl/Variable.c
@@ -29,6 +29,7 @@ EblGetCmd (
   VOID*       Value;
   CHAR8*      AsciiVariableName = NULL;
   CHAR16*     VariableName;
+  UINTN       VariableNameLen;
   UINT32      Index;
 
   if (Argc == 1) {
@@ -48,8 +49,9 @@ EblGetCmd (
     AsciiPrint("Variable name is missing.\n");
     return Status;
   } else {
-    VariableName = AllocatePool((AsciiStrLen (AsciiVariableName) + 1) * sizeof (CHAR16));
-    AsciiStrToUnicodeStr (AsciiVariableName,VariableName);
+    VariableNameLen = AsciiStrLen (AsciiVariableName) + 1;
+    VariableName = AllocatePool (VariableNameLen * sizeof (CHAR16));
+    AsciiStrToUnicodeStrS (AsciiVariableName, VariableName, VariableNameLen);
   }
 
   // Try to get the variable size.
@@ -93,6 +95,7 @@ EblSetCmd (
   CHAR8*        AsciiValue;
   UINT32        AsciiValueLength;
   CHAR16*       VariableName;
+  UINTN         VariableNameLen;
   UINT32        Index;
   UINT32        EscapedQuotes = 0;
   BOOLEAN       Volatile = FALSE;
@@ -125,8 +128,9 @@ EblSetCmd (
     //
 
     // Convert VariableName into Unicode
-    VariableName = AllocatePool((AsciiStrLen (AsciiVariableSetting) + 1) * sizeof (CHAR16));
-    AsciiStrToUnicodeStr (AsciiVariableSetting,VariableName);
+    VariableNameLen = AsciiStrLen (AsciiVariableSetting) + 1;
+    VariableName = AllocatePool (VariableNameLen * sizeof (CHAR16));
+    AsciiStrToUnicodeStrS (AsciiVariableSetting, VariableName, VariableNameLen);
 
     Status = gRT->SetVariable (
                           VariableName,
@@ -170,8 +174,9 @@ EblSetCmd (
   }
 
   // Convert VariableName into Unicode
-  VariableName = AllocatePool((AsciiStrLen (AsciiVariableName) + 1) * sizeof (CHAR16));
-  AsciiStrToUnicodeStr (AsciiVariableName,VariableName);
+  VariableNameLen = AsciiStrLen (AsciiVariableName) + 1;
+  VariableName = AllocatePool (VariableNameLen * sizeof (CHAR16));
+  AsciiStrToUnicodeStrS (AsciiVariableName, VariableName, VariableNameLen);
 
   Status = gRT->SetVariable (
                       VariableName,
-- 
2.7.4

[PATCH v2 7/9] EmbeddedPkg/EfiFileLib: eliminate deprecated string function calls

[Ard Biesheuvel]

Get rid of calls to unsafe string functions. These are deprecated and may
be removed in the future.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
---
 EmbeddedPkg/Library/EfiFileLib/EfiFileLib.c | 42 +++++++++++---------
 1 file changed, 23 insertions(+), 19 deletions(-)

diff --git a/EmbeddedPkg/Library/EfiFileLib/EfiFileLib.c b/EmbeddedPkg/Library/EfiFileLib/EfiFileLib.c
index 4d58c830861c..d3b65aa5a3e0 100644
--- a/EmbeddedPkg/Library/EfiFileLib/EfiFileLib.c
+++ b/EmbeddedPkg/Library/EfiFileLib/EfiFileLib.c
@@ -384,9 +384,9 @@ EblFileDevicePath (
 
 
   if ( *FileName != 0 ) {
-    AsciiStrToUnicodeStr (FileName, UnicodeFileName);
+    AsciiStrToUnicodeStrS (FileName, UnicodeFileName, MAX_PATHNAME);
   } else {
-    AsciiStrToUnicodeStr ("\\", UnicodeFileName);
+    AsciiStrToUnicodeStrS ("\\", UnicodeFileName, MAX_PATHNAME);
   }
 
   Size = StrSize (UnicodeFileName);
@@ -589,7 +589,7 @@ EblFvFileDevicePath (
           &AuthenticationStatus
           );
         if (!EFI_ERROR (Status)) {
-          UnicodeStrToAsciiStr (Section, AsciiSection);
+          UnicodeStrToAsciiStrS (Section, AsciiSection, MAX_PATHNAME);
           if (AsciiStriCmp (FileName, AsciiSection) == 0) {
             FreePool (Section);
             break;
@@ -674,6 +674,7 @@ EfiOpen (
   CHAR8                     *CwdPlusPathName;
   UINTN                     Index;
   EFI_SECTION_TYPE          ModifiedSectionType;
+  UINTN                     AsciiLength;
 
   EblUpdateDeviceLists ();
 
@@ -706,7 +707,8 @@ EfiOpen (
       }
 
       // We could add a current working directory concept
-      CwdPlusPathName = AllocatePool (AsciiStrSize (gCwd) + AsciiStrSize (PathName));
+      AsciiLength = AsciiStrSize (gCwd) + AsciiStrSize (PathName);
+      CwdPlusPathName = AllocatePool (AsciiLength);
       if (CwdPlusPathName == NULL) {
         return NULL;
       }
@@ -723,14 +725,14 @@ EfiOpen (
           }
         }
       } else {
-        AsciiStrCpy (CwdPlusPathName, gCwd);
+        AsciiStrCpyS (CwdPlusPathName, AsciiLength, gCwd);
         StrLen = AsciiStrLen (gCwd);
         if ((*PathName != '/') && (*PathName != '\\') && (gCwd[StrLen-1] != '/') && (gCwd[StrLen-1] != '\\')) {
-          AsciiStrCat (CwdPlusPathName, "\\");
+          AsciiStrCatS (CwdPlusPathName, AsciiLength, "\\");
         }
       }
 
-      AsciiStrCat (CwdPlusPathName, PathName);
+      AsciiStrCatS (CwdPlusPathName, AsciiLength, PathName);
       if (AsciiStrStr (CwdPlusPathName, ":") == NULL) {
         // Extra error check to make sure we don't recurse and blow stack
         return NULL;
@@ -745,7 +747,7 @@ EfiOpen (
   }
 
   File->DeviceName = AllocatePool (StrLen);
-  AsciiStrCpy (File->DeviceName, PathName);
+  AsciiStrCpyS (File->DeviceName, StrLen, PathName);
   File->DeviceName[FileStart - 1] = '\0';
   File->FileName = &File->DeviceName[FileStart];
   if (File->FileName[0] == '\0') {
@@ -1611,7 +1613,7 @@ ExpandPath (
 {
   CHAR8   *NewPath;
   CHAR8   *Work, *Start, *End;
-  UINTN   StrLen;
+  UINTN   StrLen, AllocLen;
   INTN    i;
 
   if (Cwd == NULL || Path == NULL) {
@@ -1625,11 +1627,12 @@ ExpandPath (
   }
 
   StrLen = AsciiStrSize (Path);
-  NewPath = AllocatePool (AsciiStrSize (Cwd) + StrLen + 1);
+  AllocLen = AsciiStrSize (Cwd) + StrLen + 1;
+  NewPath = AllocatePool (AllocLen);
   if (NewPath == NULL) {
     return NULL;
   }
-  AsciiStrCpy (NewPath, Cwd);
+  AsciiStrCpyS (NewPath, AllocLen, Cwd);
 
   End = Path + StrLen;
   for (Start = Path ;;) {
@@ -1640,7 +1643,7 @@ ExpandPath (
     }
 
     // append path prior to ..
-    AsciiStrnCat (NewPath, Start, Work - Start);
+    AsciiStrnCatS (NewPath, AllocLen, Start, Work - Start);
     StrLen = AsciiStrLen (NewPath);
     for (i = StrLen; i >= 0; i--) {
       if (NewPath[i] == ':') {
@@ -1663,7 +1666,7 @@ ExpandPath (
   }
 
   // Handle the path that remains after the ..
-  AsciiStrnCat (NewPath, Start, End - Start);
+  AsciiStrnCatS (NewPath, AllocLen, Start, End - Start);
 
   return NewPath;
 }
@@ -1686,7 +1689,7 @@ EfiSetCwd (
   )
 {
   EFI_OPEN_FILE *File;
-  UINTN         Len;
+  UINTN         Len, AllocLen;
   CHAR8         *Path;
 
   if (Cwd == NULL) {
@@ -1729,17 +1732,18 @@ EfiSetCwd (
 
   // Use the info returned from EfiOpen as it can add in CWD if needed. So Cwd could be
   // relative to the current gCwd or not.
-  gCwd = AllocatePool (AsciiStrSize (File->DeviceName) + AsciiStrSize (File->FileName) + 10);
+  AllocLen = AsciiStrSize (File->DeviceName) + AsciiStrSize (File->FileName) + 10;
+  gCwd = AllocatePool (AllocLen);
   if (gCwd == NULL) {
     return EFI_INVALID_PARAMETER;
   }
 
-  AsciiStrCpy (gCwd, File->DeviceName);
+  AsciiStrCpyS (gCwd, AllocLen, File->DeviceName);
   if (File->FileName == NULL) {
-    AsciiStrCat (gCwd, ":\\");
+    AsciiStrCatS (gCwd, AllocLen, ":\\");
   } else {
-    AsciiStrCat (gCwd, ":");
-    AsciiStrCat (gCwd, File->FileName);
+    AsciiStrCatS (gCwd, AllocLen, ":");
+    AsciiStrCatS (gCwd, AllocLen, File->FileName);
   }
 
 
-- 
2.7.4

[PATCH v2 8/9] EmbeddedPkg/MmcDxe: eliminate deprecated string function calls

[Ard Biesheuvel]

Get rid of calls to unsafe string functions. These are deprecated and may
be removed in the future.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 EmbeddedPkg/Universal/MmcDxe/Diagnostics.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/EmbeddedPkg/Universal/MmcDxe/Diagnostics.c b/EmbeddedPkg/Universal/MmcDxe/Diagnostics.c
index 783e548d2aed..7d6a5a0dde01 100644
--- a/EmbeddedPkg/Universal/MmcDxe/Diagnostics.c
+++ b/EmbeddedPkg/Universal/MmcDxe/Diagnostics.c
@@ -42,9 +42,9 @@ DiagnosticLog (
   )
 {
   UINTN len = StrLen (Str);
-  if (len <= mLogRemainChar) {
+  if (len < mLogRemainChar) {
+    StrCpyS (mLogBuffer, mLogRemainChar, Str);
     mLogRemainChar -= len;
-    StrCpy (mLogBuffer, Str);
     mLogBuffer += len;
     return len;
   } else {
-- 
2.7.4

[PATCH v2 9/9] EmbeddedPkg: enable -DDISABLE_NEW_DEPRECATED_INTERFACES

[Ard Biesheuvel]

Define DISABLE_NEW_DEPRECATED_INTERFACES on the compiler command line by
default, to prevent deprecated interfaces from being used in core EDK2
code.

Bug: https://bugzilla.tianocore.org/show_bug.cgi?id=164
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 EmbeddedPkg/EmbeddedPkg.dsc | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/EmbeddedPkg/EmbeddedPkg.dsc b/EmbeddedPkg/EmbeddedPkg.dsc
index cc95564f31af..eb7af800f0b2 100644
--- a/EmbeddedPkg/EmbeddedPkg.dsc
+++ b/EmbeddedPkg/EmbeddedPkg.dsc
@@ -4,6 +4,7 @@
 #
 # Copyright (c) 2007 - 2015, Intel Corporation. All rights reserved.<BR>
 # Copyright (c) 2012-2015, ARM Ltd. All rights reserved.<BR>
+# Copyright (c) 2016, Linaro Ltd. All rights reserved.<BR>
 #
 #    This program and the accompanying materials
 #    are licensed and made available under the terms and conditions of the BSD License
@@ -227,7 +228,7 @@ [PcdsFixedAtBuild.IPF]
 
 [BuildOptions]
   RVCT:*_*_ARM_PLATFORM_FLAGS == --cpu=7-A.security
-
+  *_*_*_CC_FLAGS  = -DDISABLE_NEW_DEPRECATED_INTERFACES
 
 ################################################################################
 #
-- 
2.7.4

Re: [PATCH v2 1/9] EmbeddedPkg/AndroidFastbootTransportTcpDxe: remove broken hostname handling

[Leif Lindholm]

On Fri, Oct 28, 2016 at 11:44:30AM +0100, Ard Biesheuvel wrote:
> The fastboot TCP connection setup routine retrieves a hostname from a
> UEFI variable 'hostname' that is scoped under a GUID gEfiHostnameVariableGuid
> whose definition is missing from the code. Since the hostname is only printed
> and then discarded, let's just drop the whole thing.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

It also gets rid of some needlessly hardcoded buffers, which I'm all
for - so:
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

> ---
>  EmbeddedPkg/Drivers/AndroidFastbootTransportTcpDxe/FastbootTransportTcp.c      | 23 --------------------
>  EmbeddedPkg/Drivers/AndroidFastbootTransportTcpDxe/FastbootTransportTcpDxe.inf |  3 ---
>  2 files changed, 26 deletions(-)
> 
> diff --git a/EmbeddedPkg/Drivers/AndroidFastbootTransportTcpDxe/FastbootTransportTcp.c b/EmbeddedPkg/Drivers/AndroidFastbootTransportTcpDxe/FastbootTransportTcp.c
> index 8ec78aeb534e..7c008ac9722e 100644
> --- a/EmbeddedPkg/Drivers/AndroidFastbootTransportTcpDxe/FastbootTransportTcp.c
> +++ b/EmbeddedPkg/Drivers/AndroidFastbootTransportTcpDxe/FastbootTransportTcp.c
> @@ -27,8 +27,6 @@
>  #include <Library/UefiDriverEntryPoint.h>
>  #include <Library/UefiRuntimeServicesTableLib.h>
>  
> -#include <Guid/Hostname.h>
> -
>  #define IP4_ADDR_TO_STRING(IpAddr, IpAddrString) UnicodeSPrint (       \
>                                                     IpAddrString,       \
>                                                     16 * 2,             \
> @@ -307,9 +305,6 @@ TcpFastbootTransportStart (
>    EFI_HANDLE                   *HandleBuffer;
>    EFI_IP4_MODE_DATA             Ip4ModeData;
>    UINTN                         NumHandles;
> -  UINTN                         HostnameSize = 256;
> -  CHAR8                         Hostname[256];
> -  CHAR16                        HostnameUnicode[256] = L"<no hostname>";
>    CHAR16                        IpAddrString[16];
>    UINTN                         Index;
>  
> @@ -442,28 +437,10 @@ TcpFastbootTransportStart (
>    //
>    IP4_ADDR_TO_STRING (Ip4ModeData.ConfigData.StationAddress, IpAddrString);
>  
> -  // Look up hostname
> -  Status = gRT->GetVariable (
> -                  L"Hostname",
> -                  &gEfiHostnameVariableGuid,
> -                  NULL,
> -                  &HostnameSize,
> -                  &Hostname
> -                  );
> -  if (!EFI_ERROR (Status) && HostnameSize != 0) {
> -    AsciiStrToUnicodeStr (Hostname, HostnameUnicode);
> -  }
> -
> -  // Hostname variable is not null-terminated.
> -  Hostname[HostnameSize] = L'\0';
> -
>    mTextOut->OutputString (mTextOut, L"TCP Fastboot transport configured.");
>    mTextOut->OutputString (mTextOut, L"\r\nIP address: ");
>    mTextOut->OutputString (mTextOut ,IpAddrString);
>    mTextOut->OutputString (mTextOut, L"\r\n");
> -  mTextOut->OutputString (mTextOut, L"\r\nhostname: ");
> -  mTextOut->OutputString (mTextOut, HostnameUnicode);
> -  mTextOut->OutputString (mTextOut, L"\r\n");
>  
>    //
>    // Start listening for a connection
> diff --git a/EmbeddedPkg/Drivers/AndroidFastbootTransportTcpDxe/FastbootTransportTcpDxe.inf b/EmbeddedPkg/Drivers/AndroidFastbootTransportTcpDxe/FastbootTransportTcpDxe.inf
> index 4d777934e5a4..89ff556b40ca 100644
> --- a/EmbeddedPkg/Drivers/AndroidFastbootTransportTcpDxe/FastbootTransportTcpDxe.inf
> +++ b/EmbeddedPkg/Drivers/AndroidFastbootTransportTcpDxe/FastbootTransportTcpDxe.inf
> @@ -47,8 +47,5 @@ [Packages]
>    MdeModulePkg/MdeModulePkg.dec
>    EmbeddedPkg/EmbeddedPkg.dec
>  
> -[Guids]
> -  gEfiHostnameVariableGuid
> -
>  [FixedPcd]
>    gEmbeddedTokenSpaceGuid.PcdAndroidFastbootTcpPort
> -- 
> 2.7.4
> 

Re: [PATCH v2 2/9] EmbeddedPkg: remove unused PrePiHobListPointerLib

[Leif Lindholm]

On Fri, Oct 28, 2016 at 11:44:31AM +0100, Ard Biesheuvel wrote:
> Remove this unused version: all existing platforms use the one under
> ArmPlatformPkg instead.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

> ---
>  EmbeddedPkg/EmbeddedPkg.dec                                           |  1 -
>  EmbeddedPkg/EmbeddedPkg.dsc                                           |  1 -
>  EmbeddedPkg/Library/PrePiHobListPointerLib/PrePiHobListPointer.c      | 69 --------------------
>  EmbeddedPkg/Library/PrePiHobListPointerLib/PrePiHobListPointerLib.inf | 38 -----------
>  Omap35xxPkg/Omap35xxPkg.dsc                                           |  1 -
>  5 files changed, 110 deletions(-)
> 
> diff --git a/EmbeddedPkg/EmbeddedPkg.dec b/EmbeddedPkg/EmbeddedPkg.dec
> index 775d863c5f64..2c2cf41103c2 100644
> --- a/EmbeddedPkg/EmbeddedPkg.dec
> +++ b/EmbeddedPkg/EmbeddedPkg.dec
> @@ -97,7 +97,6 @@ [PcdsFixedAtBuild.common]
>    gEmbeddedTokenSpaceGuid.PcdEmbeddedMemVariableStoreSize|0x10000|UINT32|0x00000009
>    gEmbeddedTokenSpaceGuid.PcdEmbeddedPrompt|"Ebl"|VOID*|0x00000034
>  
> -  gEmbeddedTokenSpaceGuid.PcdPrePiHobBase|131072|UINT32|0x00000040
>    gEmbeddedTokenSpaceGuid.PcdPrePiStackBase|0|UINT32|0x0000000b
>    gEmbeddedTokenSpaceGuid.PcdPrePiStackSize|131072|UINT32|0x0000000c
>  
> diff --git a/EmbeddedPkg/EmbeddedPkg.dsc b/EmbeddedPkg/EmbeddedPkg.dsc
> index cd1f6df06b24..5224bcefa9c7 100644
> --- a/EmbeddedPkg/EmbeddedPkg.dsc
> +++ b/EmbeddedPkg/EmbeddedPkg.dsc
> @@ -184,7 +184,6 @@ [PcdsFixedAtBuild.common]
>    gEmbeddedTokenSpaceGuid.PcdEmbeddedDefaultTextColor|0x07
>    gEmbeddedTokenSpaceGuid.PcdEmbeddedMemVariableStoreSize|0x10000
>  
> -  gEmbeddedTokenSpaceGuid.PcdPrePiHobBase|0
>    gEmbeddedTokenSpaceGuid.PcdPrePiStackBase|0
>    gEmbeddedTokenSpaceGuid.PcdPrePiStackSize|0
>  
> diff --git a/EmbeddedPkg/Library/PrePiHobListPointerLib/PrePiHobListPointer.c b/EmbeddedPkg/Library/PrePiHobListPointerLib/PrePiHobListPointer.c
> deleted file mode 100644
> index 9065b1132fda..000000000000
> --- a/EmbeddedPkg/Library/PrePiHobListPointerLib/PrePiHobListPointer.c
> +++ /dev/null
> @@ -1,69 +0,0 @@
> -/** @file
> -*
> -*  Copyright (c) 2011, ARM Limited. All rights reserved.
> -*
> -*  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
> -*  http://opensource.org/licenses/bsd-license.php
> -*
> -*  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> -*  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> -*
> -**/
> -
> -#include <PiPei.h>
> -#include <Library/PrePiHobListPointerLib.h>
> -#include <Library/DebugLib.h>
> -#include <Library/PcdLib.h>
> -
> -//
> -// Have to use build system to set the original value in case we are running
> -// from FLASH and globals don't work. So if you do a GetHobList() and gHobList
> -// and gHobList is NULL the PCD default values are used.
> -//
> -VOID *gHobList = NULL;
> -
> -
> -/**
> -  Returns the pointer to the HOB list.
> -
> -  This function returns the pointer to first HOB in the list.
> -
> -  @return The pointer to the HOB list.
> -
> -**/
> -VOID *
> -EFIAPI
> -PrePeiGetHobList (
> -  VOID
> -  )
> -{
> -  if (gHobList == NULL) {
> -    return (VOID *)*(UINTN*)PcdGet32 (PcdPrePiHobBase);
> -  } else {
> -    return gHobList;
> -  }
> -}
> -
> -
> -
> -/**
> -  Updates the pointer to the HOB list.
> -
> -  @param  HobList       Hob list pointer to store
> -
> -**/
> -EFI_STATUS
> -EFIAPI
> -PrePeiSetHobList (
> -  IN  VOID      *HobList
> -  )
> -{
> -  gHobList = HobList;
> -
> -  //
> -  // If this code is running from ROM this could fail
> -  //
> -  return (gHobList == HobList) ? EFI_SUCCESS: EFI_UNSUPPORTED;
> -}
> diff --git a/EmbeddedPkg/Library/PrePiHobListPointerLib/PrePiHobListPointerLib.inf b/EmbeddedPkg/Library/PrePiHobListPointerLib/PrePiHobListPointerLib.inf
> deleted file mode 100644
> index bfd3c209a13b..000000000000
> --- a/EmbeddedPkg/Library/PrePiHobListPointerLib/PrePiHobListPointerLib.inf
> +++ /dev/null
> @@ -1,38 +0,0 @@
> -#/** @file
> -#
> -#  Copyright (c) 2011, ARM Limited. All rights reserved.
> -#
> -#  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
> -#  http://opensource.org/licenses/bsd-license.php
> -#
> -#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> -#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> -#
> -#**/
> -
> -[Defines]
> -  INF_VERSION                    = 0x00010005
> -  BASE_NAME                      = PrePiHobListPointerLib
> -  FILE_GUID                      = 7a6a7fc0-5ee0-11e0-b47f-0002a5d5c51b
> -  MODULE_TYPE                    = BASE
> -  VERSION_STRING                 = 1.0
> -  LIBRARY_CLASS                  = PrePiHobListPointerLib
> -
> -#
> -#  VALID_ARCHITECTURES           = IA32 X64 IPF EBC (EBC is for build only)
> -#
> -
> -[Sources]
> -  PrePiHobListPointer.c
> -
> -[Packages]
> -  MdePkg/MdePkg.dec
> -  EmbeddedPkg/EmbeddedPkg.dec
> -
> -[LibraryClasses]
> -  #DebugLib
> -
> -[FixedPcd.common]
> -  gEmbeddedTokenSpaceGuid.PcdPrePiHobBase
> diff --git a/Omap35xxPkg/Omap35xxPkg.dsc b/Omap35xxPkg/Omap35xxPkg.dsc
> index 580be4d6340c..4a3317f255f6 100644
> --- a/Omap35xxPkg/Omap35xxPkg.dsc
> +++ b/Omap35xxPkg/Omap35xxPkg.dsc
> @@ -141,7 +141,6 @@ [PcdsFixedAtBuild.common]
>  
>    gEmbeddedTokenSpaceGuid.PcdFlashFvMainBase|0
>    gEmbeddedTokenSpaceGuid.PcdFlashFvMainSize|0
> -  gEmbeddedTokenSpaceGuid.PcdPrePiHobBase|0x80001000
>    gEmbeddedTokenSpaceGuid.PcdPrePiStackBase|0x87FE0000 # stack at top of memory
>    gEmbeddedTokenSpaceGuid.PcdPrePiStackSize|0x20000  # 128K stack
>    gArmTokenSpaceGuid.PcdCpuVectorBaseAddress|0x80000000
> -- 
> 2.7.4
> 

Re: [PATCH v2 3/9] EmbeddedPkg: add missing modules

[Leif Lindholm]

On Fri, Oct 28, 2016 at 11:44:32AM +0100, Ard Biesheuvel wrote:
> Add modules that live under EmbeddedPkg but were missing from the
> [Components] section of the package .dsc file
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

> ---
>  EmbeddedPkg/EmbeddedPkg.dsc | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/EmbeddedPkg/EmbeddedPkg.dsc b/EmbeddedPkg/EmbeddedPkg.dsc
> index 5224bcefa9c7..cc95564f31af 100644
> --- a/EmbeddedPkg/EmbeddedPkg.dsc
> +++ b/EmbeddedPkg/EmbeddedPkg.dsc
> @@ -280,5 +280,14 @@ [Components.common]
>    EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118Dxe.inf
>    EmbeddedPkg/Drivers/SataSiI3132Dxe/SataSiI3132Dxe.inf
>  
> +  EmbeddedPkg/Library/AcpiLib/AcpiLib.inf
> +  EmbeddedPkg/Library/DebugAgentTimerLibNull/DebugAgentTimerLibNull.inf
> +  EmbeddedPkg/Library/DxeHobPeCoffLib/DxeHobPeCoffLib.inf
> +  EmbeddedPkg/Library/EblNetworkLib/EblNetworkLib.inf
> +  EmbeddedPkg/Library/FdtLib/FdtLib.inf
> +  EmbeddedPkg/Library/GdbDebugAgent/GdbDebugAgent.inf
> +  EmbeddedPkg/Library/PrePiHobLib/PrePiHobLib.inf
> +  EmbeddedPkg/Library/PrePiMemoryAllocationLib/PrePiMemoryAllocationLib.inf
> +
>  [Components.IA32, Components.X64, Components.IPF, Components.ARM]
>    EmbeddedPkg/GdbStub/GdbStub.inf
> -- 
> 2.7.4
> 

Re: [PATCH v2 4/9] EmbeddedPkg/GdbDebugAgent: fix VOID* cast of incorrect size

[Leif Lindholm]

On Fri, Oct 28, 2016 at 11:44:33AM +0100, Ard Biesheuvel wrote:
> The result of PcdGet64() can only be cast to VOID* on 64-bit platforms,
> so add an intermediate UINTN cast to make this code build again on 32 bit.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> ---
>  EmbeddedPkg/Library/GdbDebugAgent/Arm/Processor.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/EmbeddedPkg/Library/GdbDebugAgent/Arm/Processor.c b/EmbeddedPkg/Library/GdbDebugAgent/Arm/Processor.c
> index 8226c80ab466..a43ff95d03a3 100644
> --- a/EmbeddedPkg/Library/GdbDebugAgent/Arm/Processor.c
> +++ b/EmbeddedPkg/Library/GdbDebugAgent/Arm/Processor.c
> @@ -657,7 +657,7 @@ InitializeDebugAgent (
>    *(UINTN *) (((UINT8 *)VectorBase) + Offset) = (UINTN)AsmCommonExceptionEntry;

Yes, I know this is just context, but *twitch*.

>  
>    // Flush Caches since we updated executable stuff
> -  InvalidateInstructionCacheRange ((VOID *)PcdGet64(PcdCpuVectorBaseAddress), Length);
> +  InvalidateInstructionCacheRange ((VOID *)(UINTN)PcdGet64(PcdCpuVectorBaseAddress), Length);
>  
>    // setup a timer so gdb can break in via ctrl-c
>    DebugAgentTimerIntialize ();
> -- 
> 2.7.4

Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

Re: [PATCH v2 5/9] EmbeddedPkg/AndroidFastboot: eliminate deprecated string function calls

[Laszlo Ersek]

On 10/28/16 12:44, Ard Biesheuvel wrote:
> Get rid of calls to unsafe string functions. These are deprecated and may
> be removed in the future.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  EmbeddedPkg/Application/AndroidFastboot/AndroidBootImg.c     |  3 ++-
>  EmbeddedPkg/Application/AndroidFastboot/AndroidFastbootApp.c | 11 ++++++-----
>  2 files changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/EmbeddedPkg/Application/AndroidFastboot/AndroidBootImg.c b/EmbeddedPkg/Application/AndroidFastboot/AndroidBootImg.c
> index bbca90fc08a2..f3e770bcc980 100644
> --- a/EmbeddedPkg/Application/AndroidFastboot/AndroidBootImg.c
> +++ b/EmbeddedPkg/Application/AndroidFastboot/AndroidBootImg.c
> @@ -84,7 +84,8 @@ ParseAndroidBootImg (
>                   + ALIGN_VALUE (Header->KernelSize, Header->PageSize));
>    }
>  
> -  AsciiStrnCpy (KernelArgs, Header->KernelArgs, BOOTIMG_KERNEL_ARGS_SIZE);
> +  AsciiStrnCpyS (KernelArgs, BOOTIMG_KERNEL_ARGS_SIZE, Header->KernelArgs,
> +    BOOTIMG_KERNEL_ARGS_SIZE);
>  
>    return EFI_SUCCESS;
>  }
> diff --git a/EmbeddedPkg/Application/AndroidFastboot/AndroidFastbootApp.c b/EmbeddedPkg/Application/AndroidFastboot/AndroidFastbootApp.c
> index 9ddc34f57cf4..c5e8a7e34af2 100644
> --- a/EmbeddedPkg/Application/AndroidFastboot/AndroidFastbootApp.c
> +++ b/EmbeddedPkg/Application/AndroidFastboot/AndroidFastbootApp.c
> @@ -99,7 +99,7 @@ HandleDownload (
>    IN CHAR8 *NumBytesString
>    )
>  {
> -  CHAR8       Response[12] = "DATA";
> +  CHAR8       Response[13];
>    CHAR16      OutputString[FASTBOOT_STRING_MAX_LENGTH];
>  
>    // Argument is 8-character ASCII string hex representation of number of bytes
> @@ -127,8 +127,10 @@ HandleDownload (
>    if (mDataBuffer == NULL) {
>      SEND_LITERAL ("FAILNot enough memory");
>    } else {
> -    AsciiStrnCpy (Response + 4, NumBytesString, 8);
> -    mTransport->Send (sizeof(Response), Response, &mFatalSendErrorEvent);
> +    ZeroMem (Response, sizeof Response);
> +    AsciiSPrint (Response, sizeof Response, "DATA%x",
> +      (UINT32)mNumDataBytes);
> +    mTransport->Send (sizeof Response - 1, Response, &mFatalSendErrorEvent);
>  
>      mState = ExpectDataState;
>      mBytesReceivedSoFar = 0;
> @@ -257,8 +259,7 @@ AcceptCmd (
>    }
>  
>    // Commands aren't null-terminated. Let's get a null-terminated version.
> -  AsciiStrnCpy (Command, Data, Size);
> -  Command[Size] = '\0';
> +  AsciiStrnCpyS (Command, sizeof Command, Data, Size);
>  
>    // Parse command
>    if (MATCH_CMD_LITERAL ("getvar", Command)) {
> 

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Re: [PATCH v2 6/9] EmbeddedPkg/Ebl: eliminate deprecated string function calls

[Laszlo Ersek]

On 10/28/16 12:44, Ard Biesheuvel wrote:
> Get rid of calls to unsafe string functions. These are deprecated and may
> be removed in the future.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  EmbeddedPkg/Ebl/Command.c   |  2 +-
>  EmbeddedPkg/Ebl/Dir.c       |  4 ++--
>  EmbeddedPkg/Ebl/EfiDevice.c | 11 ++++++-----
>  EmbeddedPkg/Ebl/Main.c      |  8 ++++----
>  EmbeddedPkg/Ebl/Variable.c  | 17 +++++++++++------
>  5 files changed, 24 insertions(+), 18 deletions(-)
> 
> diff --git a/EmbeddedPkg/Ebl/Command.c b/EmbeddedPkg/Ebl/Command.c
> index e75c6a2e5c32..4bc1f4df0ca0 100644
> --- a/EmbeddedPkg/Ebl/Command.c
> +++ b/EmbeddedPkg/Ebl/Command.c
> @@ -614,7 +614,7 @@ OutputData (
>    UINTN Spaces   = 0;
>    CHAR8 Blanks[80];
>  
> -  AsciiStrCpy (Blanks, mBlanks);
> +  AsciiStrCpyS (Blanks, sizeof Blanks, mBlanks);
>    for (EndAddress = Address + Length; Address < EndAddress; Offset += Line) {
>      AsciiPrint ("%08x: ", Offset);
>      for (Line = 0; (Line < 0x10) && (Address < EndAddress);) {
> diff --git a/EmbeddedPkg/Ebl/Dir.c b/EmbeddedPkg/Ebl/Dir.c
> index 36095b633019..8dd9d48ff6ac 100644
> --- a/EmbeddedPkg/Ebl/Dir.c
> +++ b/EmbeddedPkg/Ebl/Dir.c
> @@ -116,7 +116,7 @@ EblDirCmd (
>      UnicodeFileName[0] = '\0';
>      MatchSubString = &UnicodeFileName[0];
>      if (Argc > 2) {
> -      AsciiStrToUnicodeStr (Argv[2], UnicodeFileName);
> +      AsciiStrToUnicodeStrS (Argv[2], UnicodeFileName, MAX_CMD_LINE);
>        if (UnicodeFileName[0] == '*') {
>          // Handle *Name substring matching
>          MatchSubString = &UnicodeFileName[1];
> @@ -231,7 +231,7 @@ EblDirCmd (
>      MatchSubString = NULL;
>      UnicodeFileName[0] = '\0';
>      if (Argc > 2) {
> -      AsciiStrToUnicodeStr (Argv[2], UnicodeFileName);
> +      AsciiStrToUnicodeStrS (Argv[2], UnicodeFileName, MAX_CMD_LINE);
>        if (UnicodeFileName[0] == '*') {
>          MatchSubString = &UnicodeFileName[1];
>        }
> diff --git a/EmbeddedPkg/Ebl/EfiDevice.c b/EmbeddedPkg/Ebl/EfiDevice.c
> index ec9c331b7004..f6969e7b2b05 100644
> --- a/EmbeddedPkg/Ebl/EfiDevice.c
> +++ b/EmbeddedPkg/Ebl/EfiDevice.c
> @@ -343,7 +343,7 @@ EblStartCmd (
>  
>        ImageInfo->LoadOptionsSize = (UINT32)AsciiStrSize (Argv[2]);
>        ImageInfo->LoadOptions     = AllocatePool (ImageInfo->LoadOptionsSize);
> -      AsciiStrCpy (ImageInfo->LoadOptions, Argv[2]);
> +      AsciiStrCpyS (ImageInfo->LoadOptions, ImageInfo->LoadOptionsSize, Argv[2]);
>      }
>  
>      // Transfer control to the EFI image we loaded with LoadImage()
> @@ -741,7 +741,7 @@ EblFileCopyCmd (
>    UINTN         Size;
>    UINTN         Offset;
>    UINTN         Chunk        = FILE_COPY_CHUNK;
> -  UINTN         FileNameLen;
> +  UINTN         FileNameLen, DestFileNameLen;
>    CHAR8*        DestFileName;
>    CHAR8*        SrcFileName;
>    CHAR8*        SrcPtr;
> @@ -786,9 +786,10 @@ EblFileCopyCmd (
>      }
>  
>      // Construct the destination filepath
> -    DestFileName = (CHAR8*)AllocatePool (FileNameLen + AsciiStrLen (SrcFileName) + 1);
> -    AsciiStrCpy (DestFileName, Argv[2]);
> -    AsciiStrCat (DestFileName, SrcFileName);
> +    DestFileNameLen = FileNameLen + AsciiStrLen (SrcFileName) + 1;
> +    DestFileName = (CHAR8*)AllocatePool (DestFileNameLen);
> +    AsciiStrCpyS (DestFileName, DestFileNameLen, Argv[2]);
> +    AsciiStrCatS (DestFileName, DestFileNameLen, SrcFileName);
>    }
>  
>    Source = EfiOpen(Argv[1], EFI_FILE_MODE_READ, 0);
> diff --git a/EmbeddedPkg/Ebl/Main.c b/EmbeddedPkg/Ebl/Main.c
> index 18b2878f69a1..62f559fccfe8 100644
> --- a/EmbeddedPkg/Ebl/Main.c
> +++ b/EmbeddedPkg/Ebl/Main.c
> @@ -88,7 +88,7 @@ SetCmdHistory (
>      }
>  
>      // Copy the new command line into the ring buffer
> -    AsciiStrnCpy(&mCmdHistory[mCmdHistoryStart][0], Cmd, MAX_CMD_LINE);
> +    AsciiStrnCpyS (&mCmdHistory[mCmdHistoryStart][0], MAX_CMD_LINE, Cmd, MAX_CMD_LINE);
>    }
>  
>    // Reset the command history for the next up arrow press
> @@ -432,7 +432,7 @@ GetCmd (
>        }
>        AsciiPrint (History);
>        Index = AsciiStrLen (History);
> -      AsciiStrnCpy (Cmd, History, CmdMaxSize);
> +      AsciiStrnCpyS (Cmd, CmdMaxSize, History, CmdMaxSize);
>      } else {
>        Cmd[Index++] = Char;
>        if (FixedPcdGetBool(PcdEmbeddedShellCharacterEcho) == TRUE) {
> @@ -644,14 +644,14 @@ EdkBootLoaderEntry (
>  
>      Status = gRT->GetVariable(CommandLineVariableName, &VendorGuid, NULL, &CommandLineVariableSize, CommandLineVariable);
>      if (!EFI_ERROR(Status)) {
> -      UnicodeStrToAsciiStr(CommandLineVariable, CmdLine);
> +      UnicodeStrToAsciiStrS (CommandLineVariable, CmdLine, MAX_CMD_LINE);
>      }
>  
>      FreePool(CommandLineVariable);
>    }
>  
>    if (EFI_ERROR(Status)) {
> -    AsciiStrCpy (CmdLine, (CHAR8 *)PcdGetPtr (PcdEmbeddedAutomaticBootCommand));
> +    AsciiStrCpyS (CmdLine, MAX_CMD_LINE, (CHAR8 *)PcdGetPtr (PcdEmbeddedAutomaticBootCommand));
>    }
>  
>    for (;;) {
> diff --git a/EmbeddedPkg/Ebl/Variable.c b/EmbeddedPkg/Ebl/Variable.c
> index f440c48f16dd..92464a6b7133 100644
> --- a/EmbeddedPkg/Ebl/Variable.c
> +++ b/EmbeddedPkg/Ebl/Variable.c
> @@ -29,6 +29,7 @@ EblGetCmd (
>    VOID*       Value;
>    CHAR8*      AsciiVariableName = NULL;
>    CHAR16*     VariableName;
> +  UINTN       VariableNameLen;
>    UINT32      Index;
>  
>    if (Argc == 1) {
> @@ -48,8 +49,9 @@ EblGetCmd (
>      AsciiPrint("Variable name is missing.\n");
>      return Status;
>    } else {
> -    VariableName = AllocatePool((AsciiStrLen (AsciiVariableName) + 1) * sizeof (CHAR16));
> -    AsciiStrToUnicodeStr (AsciiVariableName,VariableName);
> +    VariableNameLen = AsciiStrLen (AsciiVariableName) + 1;
> +    VariableName = AllocatePool (VariableNameLen * sizeof (CHAR16));
> +    AsciiStrToUnicodeStrS (AsciiVariableName, VariableName, VariableNameLen);
>    }
>  
>    // Try to get the variable size.
> @@ -93,6 +95,7 @@ EblSetCmd (
>    CHAR8*        AsciiValue;
>    UINT32        AsciiValueLength;
>    CHAR16*       VariableName;
> +  UINTN         VariableNameLen;
>    UINT32        Index;
>    UINT32        EscapedQuotes = 0;
>    BOOLEAN       Volatile = FALSE;
> @@ -125,8 +128,9 @@ EblSetCmd (
>      //
>  
>      // Convert VariableName into Unicode
> -    VariableName = AllocatePool((AsciiStrLen (AsciiVariableSetting) + 1) * sizeof (CHAR16));
> -    AsciiStrToUnicodeStr (AsciiVariableSetting,VariableName);
> +    VariableNameLen = AsciiStrLen (AsciiVariableSetting) + 1;
> +    VariableName = AllocatePool (VariableNameLen * sizeof (CHAR16));
> +    AsciiStrToUnicodeStrS (AsciiVariableSetting, VariableName, VariableNameLen);
>  
>      Status = gRT->SetVariable (
>                            VariableName,
> @@ -170,8 +174,9 @@ EblSetCmd (
>    }
>  
>    // Convert VariableName into Unicode
> -  VariableName = AllocatePool((AsciiStrLen (AsciiVariableName) + 1) * sizeof (CHAR16));
> -  AsciiStrToUnicodeStr (AsciiVariableName,VariableName);
> +  VariableNameLen = AsciiStrLen (AsciiVariableName) + 1;
> +  VariableName = AllocatePool (VariableNameLen * sizeof (CHAR16));
> +  AsciiStrToUnicodeStrS (AsciiVariableName, VariableName, VariableNameLen);
>  
>    Status = gRT->SetVariable (
>                        VariableName,
> 

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Re: [PATCH v2 5/9] EmbeddedPkg/AndroidFastboot: eliminate deprecated string function calls

[Leif Lindholm]

On Fri, Oct 28, 2016 at 11:44:34AM +0100, Ard Biesheuvel wrote:
> Get rid of calls to unsafe string functions. These are deprecated and may
> be removed in the future.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  EmbeddedPkg/Application/AndroidFastboot/AndroidBootImg.c     |  3 ++-
>  EmbeddedPkg/Application/AndroidFastboot/AndroidFastbootApp.c | 11 ++++++-----
>  2 files changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/EmbeddedPkg/Application/AndroidFastboot/AndroidBootImg.c b/EmbeddedPkg/Application/AndroidFastboot/AndroidBootImg.c
> index bbca90fc08a2..f3e770bcc980 100644
> --- a/EmbeddedPkg/Application/AndroidFastboot/AndroidBootImg.c
> +++ b/EmbeddedPkg/Application/AndroidFastboot/AndroidBootImg.c
> @@ -84,7 +84,8 @@ ParseAndroidBootImg (
>                   + ALIGN_VALUE (Header->KernelSize, Header->PageSize));
>    }
>  
> -  AsciiStrnCpy (KernelArgs, Header->KernelArgs, BOOTIMG_KERNEL_ARGS_SIZE);
> +  AsciiStrnCpyS (KernelArgs, BOOTIMG_KERNEL_ARGS_SIZE, Header->KernelArgs,
> +    BOOTIMG_KERNEL_ARGS_SIZE);
>  
>    return EFI_SUCCESS;
>  }
> diff --git a/EmbeddedPkg/Application/AndroidFastboot/AndroidFastbootApp.c b/EmbeddedPkg/Application/AndroidFastboot/AndroidFastbootApp.c
> index 9ddc34f57cf4..c5e8a7e34af2 100644
> --- a/EmbeddedPkg/Application/AndroidFastboot/AndroidFastbootApp.c
> +++ b/EmbeddedPkg/Application/AndroidFastboot/AndroidFastbootApp.c
> @@ -99,7 +99,7 @@ HandleDownload (
>    IN CHAR8 *NumBytesString
>    )
>  {
> -  CHAR8       Response[12] = "DATA";
> +  CHAR8       Response[13];
>    CHAR16      OutputString[FASTBOOT_STRING_MAX_LENGTH];
>  
>    // Argument is 8-character ASCII string hex representation of number of bytes
> @@ -127,8 +127,10 @@ HandleDownload (
>    if (mDataBuffer == NULL) {
>      SEND_LITERAL ("FAILNot enough memory");
>    } else {
> -    AsciiStrnCpy (Response + 4, NumBytesString, 8);
> -    mTransport->Send (sizeof(Response), Response, &mFatalSendErrorEvent);
> +    ZeroMem (Response, sizeof Response);
> +    AsciiSPrint (Response, sizeof Response, "DATA%x",
> +      (UINT32)mNumDataBytes);

I'll try to keep the bikeshedding to a minimum, but since
mNumDataBytes is generated from NumBytesString in the first place, why
not do
  "DATA%s", NumBytesString
?

> +    mTransport->Send (sizeof Response - 1, Response, &mFatalSendErrorEvent);
>  
>      mState = ExpectDataState;
>      mBytesReceivedSoFar = 0;
> @@ -257,8 +259,7 @@ AcceptCmd (
>    }
>  
>    // Commands aren't null-terminated. Let's get a null-terminated version.
> -  AsciiStrnCpy (Command, Data, Size);
> -  Command[Size] = '\0';
> +  AsciiStrnCpyS (Command, sizeof Command, Data, Size);
>  
>    // Parse command
>    if (MATCH_CMD_LITERAL ("getvar", Command)) {
> -- 
> 2.7.4
> 

Re: [PATCH v2 8/9] EmbeddedPkg/MmcDxe: eliminate deprecated string function calls

[Laszlo Ersek]

On 10/28/16 12:44, Ard Biesheuvel wrote:
> Get rid of calls to unsafe string functions. These are deprecated and may
> be removed in the future.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  EmbeddedPkg/Universal/MmcDxe/Diagnostics.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/EmbeddedPkg/Universal/MmcDxe/Diagnostics.c b/EmbeddedPkg/Universal/MmcDxe/Diagnostics.c
> index 783e548d2aed..7d6a5a0dde01 100644
> --- a/EmbeddedPkg/Universal/MmcDxe/Diagnostics.c
> +++ b/EmbeddedPkg/Universal/MmcDxe/Diagnostics.c
> @@ -42,9 +42,9 @@ DiagnosticLog (
>    )
>  {
>    UINTN len = StrLen (Str);
> -  if (len <= mLogRemainChar) {
> +  if (len < mLogRemainChar) {
> +    StrCpyS (mLogBuffer, mLogRemainChar, Str);
>      mLogRemainChar -= len;
> -    StrCpy (mLogBuffer, Str);
>      mLogBuffer += len;
>      return len;
>    } else {
> 

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Re: [PATCH v2 5/9] EmbeddedPkg/AndroidFastboot: eliminate deprecated string function calls

[Ard Biesheuvel]

On 28 October 2016 at 14:36, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> On Fri, Oct 28, 2016 at 11:44:34AM +0100, Ard Biesheuvel wrote:
>> Get rid of calls to unsafe string functions. These are deprecated and may
>> be removed in the future.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>>  EmbeddedPkg/Application/AndroidFastboot/AndroidBootImg.c     |  3 ++-
>>  EmbeddedPkg/Application/AndroidFastboot/AndroidFastbootApp.c | 11 ++++++-----
>>  2 files changed, 8 insertions(+), 6 deletions(-)
>>
>> diff --git a/EmbeddedPkg/Application/AndroidFastboot/AndroidBootImg.c b/EmbeddedPkg/Application/AndroidFastboot/AndroidBootImg.c
>> index bbca90fc08a2..f3e770bcc980 100644
>> --- a/EmbeddedPkg/Application/AndroidFastboot/AndroidBootImg.c
>> +++ b/EmbeddedPkg/Application/AndroidFastboot/AndroidBootImg.c
>> @@ -84,7 +84,8 @@ ParseAndroidBootImg (
>>                   + ALIGN_VALUE (Header->KernelSize, Header->PageSize));
>>    }
>>
>> -  AsciiStrnCpy (KernelArgs, Header->KernelArgs, BOOTIMG_KERNEL_ARGS_SIZE);
>> +  AsciiStrnCpyS (KernelArgs, BOOTIMG_KERNEL_ARGS_SIZE, Header->KernelArgs,
>> +    BOOTIMG_KERNEL_ARGS_SIZE);
>>
>>    return EFI_SUCCESS;
>>  }
>> diff --git a/EmbeddedPkg/Application/AndroidFastboot/AndroidFastbootApp.c b/EmbeddedPkg/Application/AndroidFastboot/AndroidFastbootApp.c
>> index 9ddc34f57cf4..c5e8a7e34af2 100644
>> --- a/EmbeddedPkg/Application/AndroidFastboot/AndroidFastbootApp.c
>> +++ b/EmbeddedPkg/Application/AndroidFastboot/AndroidFastbootApp.c
>> @@ -99,7 +99,7 @@ HandleDownload (
>>    IN CHAR8 *NumBytesString
>>    )
>>  {
>> -  CHAR8       Response[12] = "DATA";
>> +  CHAR8       Response[13];
>>    CHAR16      OutputString[FASTBOOT_STRING_MAX_LENGTH];
>>
>>    // Argument is 8-character ASCII string hex representation of number of bytes
>> @@ -127,8 +127,10 @@ HandleDownload (
>>    if (mDataBuffer == NULL) {
>>      SEND_LITERAL ("FAILNot enough memory");
>>    } else {
>> -    AsciiStrnCpy (Response + 4, NumBytesString, 8);
>> -    mTransport->Send (sizeof(Response), Response, &mFatalSendErrorEvent);
>> +    ZeroMem (Response, sizeof Response);
>> +    AsciiSPrint (Response, sizeof Response, "DATA%x",
>> +      (UINT32)mNumDataBytes);
>
> I'll try to keep the bikeshedding to a minimum, but since
> mNumDataBytes is generated from NumBytesString in the first place, why
> not do
>   "DATA%s", NumBytesString
> ?
>

Are you asking me? Or the author of the original code?

>> +    mTransport->Send (sizeof Response - 1, Response, &mFatalSendErrorEvent);
>>
>>      mState = ExpectDataState;
>>      mBytesReceivedSoFar = 0;
>> @@ -257,8 +259,7 @@ AcceptCmd (
>>    }
>>
>>    // Commands aren't null-terminated. Let's get a null-terminated version.
>> -  AsciiStrnCpy (Command, Data, Size);
>> -  Command[Size] = '\0';
>> +  AsciiStrnCpyS (Command, sizeof Command, Data, Size);
>>
>>    // Parse command
>>    if (MATCH_CMD_LITERAL ("getvar", Command)) {
>> --
>> 2.7.4
>>

Re: [PATCH v2 5/9] EmbeddedPkg/AndroidFastboot: eliminate deprecated string function calls

[Laszlo Ersek]

On 10/28/16 15:36, Leif Lindholm wrote:
> On Fri, Oct 28, 2016 at 11:44:34AM +0100, Ard Biesheuvel wrote:
>> Get rid of calls to unsafe string functions. These are deprecated and may
>> be removed in the future.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>>  EmbeddedPkg/Application/AndroidFastboot/AndroidBootImg.c     |  3 ++-
>>  EmbeddedPkg/Application/AndroidFastboot/AndroidFastbootApp.c | 11 ++++++-----
>>  2 files changed, 8 insertions(+), 6 deletions(-)
>>
>> diff --git a/EmbeddedPkg/Application/AndroidFastboot/AndroidBootImg.c b/EmbeddedPkg/Application/AndroidFastboot/AndroidBootImg.c
>> index bbca90fc08a2..f3e770bcc980 100644
>> --- a/EmbeddedPkg/Application/AndroidFastboot/AndroidBootImg.c
>> +++ b/EmbeddedPkg/Application/AndroidFastboot/AndroidBootImg.c
>> @@ -84,7 +84,8 @@ ParseAndroidBootImg (
>>                   + ALIGN_VALUE (Header->KernelSize, Header->PageSize));
>>    }
>>  
>> -  AsciiStrnCpy (KernelArgs, Header->KernelArgs, BOOTIMG_KERNEL_ARGS_SIZE);
>> +  AsciiStrnCpyS (KernelArgs, BOOTIMG_KERNEL_ARGS_SIZE, Header->KernelArgs,
>> +    BOOTIMG_KERNEL_ARGS_SIZE);
>>  
>>    return EFI_SUCCESS;
>>  }
>> diff --git a/EmbeddedPkg/Application/AndroidFastboot/AndroidFastbootApp.c b/EmbeddedPkg/Application/AndroidFastboot/AndroidFastbootApp.c
>> index 9ddc34f57cf4..c5e8a7e34af2 100644
>> --- a/EmbeddedPkg/Application/AndroidFastboot/AndroidFastbootApp.c
>> +++ b/EmbeddedPkg/Application/AndroidFastboot/AndroidFastbootApp.c
>> @@ -99,7 +99,7 @@ HandleDownload (
>>    IN CHAR8 *NumBytesString
>>    )
>>  {
>> -  CHAR8       Response[12] = "DATA";
>> +  CHAR8       Response[13];
>>    CHAR16      OutputString[FASTBOOT_STRING_MAX_LENGTH];
>>  
>>    // Argument is 8-character ASCII string hex representation of number of bytes
>> @@ -127,8 +127,10 @@ HandleDownload (
>>    if (mDataBuffer == NULL) {
>>      SEND_LITERAL ("FAILNot enough memory");
>>    } else {
>> -    AsciiStrnCpy (Response + 4, NumBytesString, 8);
>> -    mTransport->Send (sizeof(Response), Response, &mFatalSendErrorEvent);
>> +    ZeroMem (Response, sizeof Response);
>> +    AsciiSPrint (Response, sizeof Response, "DATA%x",
>> +      (UINT32)mNumDataBytes);
> 
> I'll try to keep the bikeshedding to a minimum, but since
> mNumDataBytes is generated from NumBytesString in the first place, why
> not do
>   "DATA%s", NumBytesString
> ?

Because of "0000000000000000000000000000000000000000000000012".

Thanks
Laszlo

Re: [PATCH v2 5/9] EmbeddedPkg/AndroidFastboot: eliminate deprecated string function calls

[Leif Lindholm]

On Fri, Oct 28, 2016 at 02:40:59PM +0100, Ard Biesheuvel wrote:
> On 28 October 2016 at 14:36, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> > On Fri, Oct 28, 2016 at 11:44:34AM +0100, Ard Biesheuvel wrote:
> >> Get rid of calls to unsafe string functions. These are deprecated and may
> >> be removed in the future.
> >>
> >> Contributed-under: TianoCore Contribution Agreement 1.0
> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> >> ---
> >>  EmbeddedPkg/Application/AndroidFastboot/AndroidBootImg.c     |  3 ++-
> >>  EmbeddedPkg/Application/AndroidFastboot/AndroidFastbootApp.c | 11 ++++++-----
> >>  2 files changed, 8 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/EmbeddedPkg/Application/AndroidFastboot/AndroidBootImg.c b/EmbeddedPkg/Application/AndroidFastboot/AndroidBootImg.c
> >> index bbca90fc08a2..f3e770bcc980 100644
> >> --- a/EmbeddedPkg/Application/AndroidFastboot/AndroidBootImg.c
> >> +++ b/EmbeddedPkg/Application/AndroidFastboot/AndroidBootImg.c
> >> @@ -84,7 +84,8 @@ ParseAndroidBootImg (
> >>                   + ALIGN_VALUE (Header->KernelSize, Header->PageSize));
> >>    }
> >>
> >> -  AsciiStrnCpy (KernelArgs, Header->KernelArgs, BOOTIMG_KERNEL_ARGS_SIZE);
> >> +  AsciiStrnCpyS (KernelArgs, BOOTIMG_KERNEL_ARGS_SIZE, Header->KernelArgs,
> >> +    BOOTIMG_KERNEL_ARGS_SIZE);
> >>
> >>    return EFI_SUCCESS;
> >>  }
> >> diff --git a/EmbeddedPkg/Application/AndroidFastboot/AndroidFastbootApp.c b/EmbeddedPkg/Application/AndroidFastboot/AndroidFastbootApp.c
> >> index 9ddc34f57cf4..c5e8a7e34af2 100644
> >> --- a/EmbeddedPkg/Application/AndroidFastboot/AndroidFastbootApp.c
> >> +++ b/EmbeddedPkg/Application/AndroidFastboot/AndroidFastbootApp.c
> >> @@ -99,7 +99,7 @@ HandleDownload (
> >>    IN CHAR8 *NumBytesString
> >>    )
> >>  {
> >> -  CHAR8       Response[12] = "DATA";
> >> +  CHAR8       Response[13];
> >>    CHAR16      OutputString[FASTBOOT_STRING_MAX_LENGTH];
> >>
> >>    // Argument is 8-character ASCII string hex representation of number of bytes
> >> @@ -127,8 +127,10 @@ HandleDownload (
> >>    if (mDataBuffer == NULL) {
> >>      SEND_LITERAL ("FAILNot enough memory");
> >>    } else {
> >> -    AsciiStrnCpy (Response + 4, NumBytesString, 8);
> >> -    mTransport->Send (sizeof(Response), Response, &mFatalSendErrorEvent);
> >> +    ZeroMem (Response, sizeof Response);
> >> +    AsciiSPrint (Response, sizeof Response, "DATA%x",
> >> +      (UINT32)mNumDataBytes);
> >
> > I'll try to keep the bikeshedding to a minimum, but since
> > mNumDataBytes is generated from NumBytesString in the first place, why
> > not do
> >   "DATA%s", NumBytesString
> > ?
> >
> 
> Are you asking me? Or the author of the original code?

Well, the original code used NumBytesString, and your updated version
does not.

As per Laszlo's comment - the implementation of
AsciiStrHexToUint64 means that an arbitrarily long string of leading
zeroes could be handled by this version that would not previously have
been handled.

If that is desired behaviour, then that makes this change a bugfix
rather than just an API cleanup. Which should be mentioned in the
commit message. If you do that:

Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

/
    Leif

> >> +    mTransport->Send (sizeof Response - 1, Response, &mFatalSendErrorEvent);
> >>
> >>      mState = ExpectDataState;
> >>      mBytesReceivedSoFar = 0;
> >> @@ -257,8 +259,7 @@ AcceptCmd (
> >>    }
> >>
> >>    // Commands aren't null-terminated. Let's get a null-terminated version.
> >> -  AsciiStrnCpy (Command, Data, Size);
> >> -  Command[Size] = '\0';
> >> +  AsciiStrnCpyS (Command, sizeof Command, Data, Size);
> >>
> >>    // Parse command
> >>    if (MATCH_CMD_LITERAL ("getvar", Command)) {
> >> --
> >> 2.7.4
> >>

Re: [PATCH v2 5/9] EmbeddedPkg/AndroidFastboot: eliminate deprecated string function calls

[Ard Biesheuvel]

On 28 October 2016 at 14:52, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> On Fri, Oct 28, 2016 at 02:40:59PM +0100, Ard Biesheuvel wrote:
>> On 28 October 2016 at 14:36, Leif Lindholm <leif.lindholm@linaro.org> wrote:
>> > On Fri, Oct 28, 2016 at 11:44:34AM +0100, Ard Biesheuvel wrote:
>> >> Get rid of calls to unsafe string functions. These are deprecated and may
>> >> be removed in the future.
>> >>
>> >> Contributed-under: TianoCore Contribution Agreement 1.0
>> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> >> ---
>> >>  EmbeddedPkg/Application/AndroidFastboot/AndroidBootImg.c     |  3 ++-
>> >>  EmbeddedPkg/Application/AndroidFastboot/AndroidFastbootApp.c | 11 ++++++-----
>> >>  2 files changed, 8 insertions(+), 6 deletions(-)
>> >>
>> >> diff --git a/EmbeddedPkg/Application/AndroidFastboot/AndroidBootImg.c b/EmbeddedPkg/Application/AndroidFastboot/AndroidBootImg.c
>> >> index bbca90fc08a2..f3e770bcc980 100644
>> >> --- a/EmbeddedPkg/Application/AndroidFastboot/AndroidBootImg.c
>> >> +++ b/EmbeddedPkg/Application/AndroidFastboot/AndroidBootImg.c
>> >> @@ -84,7 +84,8 @@ ParseAndroidBootImg (
>> >>                   + ALIGN_VALUE (Header->KernelSize, Header->PageSize));
>> >>    }
>> >>
>> >> -  AsciiStrnCpy (KernelArgs, Header->KernelArgs, BOOTIMG_KERNEL_ARGS_SIZE);
>> >> +  AsciiStrnCpyS (KernelArgs, BOOTIMG_KERNEL_ARGS_SIZE, Header->KernelArgs,
>> >> +    BOOTIMG_KERNEL_ARGS_SIZE);
>> >>
>> >>    return EFI_SUCCESS;
>> >>  }
>> >> diff --git a/EmbeddedPkg/Application/AndroidFastboot/AndroidFastbootApp.c b/EmbeddedPkg/Application/AndroidFastboot/AndroidFastbootApp.c
>> >> index 9ddc34f57cf4..c5e8a7e34af2 100644
>> >> --- a/EmbeddedPkg/Application/AndroidFastboot/AndroidFastbootApp.c
>> >> +++ b/EmbeddedPkg/Application/AndroidFastboot/AndroidFastbootApp.c
>> >> @@ -99,7 +99,7 @@ HandleDownload (
>> >>    IN CHAR8 *NumBytesString
>> >>    )
>> >>  {
>> >> -  CHAR8       Response[12] = "DATA";
>> >> +  CHAR8       Response[13];
>> >>    CHAR16      OutputString[FASTBOOT_STRING_MAX_LENGTH];
>> >>
>> >>    // Argument is 8-character ASCII string hex representation of number of bytes
>> >> @@ -127,8 +127,10 @@ HandleDownload (
>> >>    if (mDataBuffer == NULL) {
>> >>      SEND_LITERAL ("FAILNot enough memory");
>> >>    } else {
>> >> -    AsciiStrnCpy (Response + 4, NumBytesString, 8);
>> >> -    mTransport->Send (sizeof(Response), Response, &mFatalSendErrorEvent);
>> >> +    ZeroMem (Response, sizeof Response);
>> >> +    AsciiSPrint (Response, sizeof Response, "DATA%x",
>> >> +      (UINT32)mNumDataBytes);
>> >
>> > I'll try to keep the bikeshedding to a minimum, but since
>> > mNumDataBytes is generated from NumBytesString in the first place, why
>> > not do
>> >   "DATA%s", NumBytesString
>> > ?
>> >
>>
>> Are you asking me? Or the author of the original code?
>
> Well, the original code used NumBytesString, and your updated version
> does not.
>

Indeed, I missed that. Been looking at many different variations on
this theme over the past week, so my vision got a little blurry,
sorry.

> As per Laszlo's comment - the implementation of
> AsciiStrHexToUint64 means that an arbitrarily long string of leading
> zeroes could be handled by this version that would not previously have
> been handled.
>
> If that is desired behaviour, then that makes this change a bugfix
> rather than just an API cleanup. Which should be mentioned in the
> commit message. If you do that:
>
> Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
>

Thanks.

Re: [PATCH v2 5/9] EmbeddedPkg/AndroidFastboot: eliminate deprecated string function calls

[Laszlo Ersek]

On 10/28/16 15:52, Leif Lindholm wrote:
> On Fri, Oct 28, 2016 at 02:40:59PM +0100, Ard Biesheuvel wrote:
>> On 28 October 2016 at 14:36, Leif Lindholm <leif.lindholm@linaro.org> wrote:
>>> On Fri, Oct 28, 2016 at 11:44:34AM +0100, Ard Biesheuvel wrote:
>>>> Get rid of calls to unsafe string functions. These are deprecated and may
>>>> be removed in the future.
>>>>
>>>> Contributed-under: TianoCore Contribution Agreement 1.0
>>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>>> ---
>>>>  EmbeddedPkg/Application/AndroidFastboot/AndroidBootImg.c     |  3 ++-
>>>>  EmbeddedPkg/Application/AndroidFastboot/AndroidFastbootApp.c | 11 ++++++-----
>>>>  2 files changed, 8 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/EmbeddedPkg/Application/AndroidFastboot/AndroidBootImg.c b/EmbeddedPkg/Application/AndroidFastboot/AndroidBootImg.c
>>>> index bbca90fc08a2..f3e770bcc980 100644
>>>> --- a/EmbeddedPkg/Application/AndroidFastboot/AndroidBootImg.c
>>>> +++ b/EmbeddedPkg/Application/AndroidFastboot/AndroidBootImg.c
>>>> @@ -84,7 +84,8 @@ ParseAndroidBootImg (
>>>>                   + ALIGN_VALUE (Header->KernelSize, Header->PageSize));
>>>>    }
>>>>
>>>> -  AsciiStrnCpy (KernelArgs, Header->KernelArgs, BOOTIMG_KERNEL_ARGS_SIZE);
>>>> +  AsciiStrnCpyS (KernelArgs, BOOTIMG_KERNEL_ARGS_SIZE, Header->KernelArgs,
>>>> +    BOOTIMG_KERNEL_ARGS_SIZE);
>>>>
>>>>    return EFI_SUCCESS;
>>>>  }
>>>> diff --git a/EmbeddedPkg/Application/AndroidFastboot/AndroidFastbootApp.c b/EmbeddedPkg/Application/AndroidFastboot/AndroidFastbootApp.c
>>>> index 9ddc34f57cf4..c5e8a7e34af2 100644
>>>> --- a/EmbeddedPkg/Application/AndroidFastboot/AndroidFastbootApp.c
>>>> +++ b/EmbeddedPkg/Application/AndroidFastboot/AndroidFastbootApp.c
>>>> @@ -99,7 +99,7 @@ HandleDownload (
>>>>    IN CHAR8 *NumBytesString
>>>>    )
>>>>  {
>>>> -  CHAR8       Response[12] = "DATA";
>>>> +  CHAR8       Response[13];
>>>>    CHAR16      OutputString[FASTBOOT_STRING_MAX_LENGTH];
>>>>
>>>>    // Argument is 8-character ASCII string hex representation of number of bytes
>>>> @@ -127,8 +127,10 @@ HandleDownload (
>>>>    if (mDataBuffer == NULL) {
>>>>      SEND_LITERAL ("FAILNot enough memory");
>>>>    } else {
>>>> -    AsciiStrnCpy (Response + 4, NumBytesString, 8);
>>>> -    mTransport->Send (sizeof(Response), Response, &mFatalSendErrorEvent);
>>>> +    ZeroMem (Response, sizeof Response);
>>>> +    AsciiSPrint (Response, sizeof Response, "DATA%x",
>>>> +      (UINT32)mNumDataBytes);
>>>
>>> I'll try to keep the bikeshedding to a minimum, but since
>>> mNumDataBytes is generated from NumBytesString in the first place, why
>>> not do
>>>   "DATA%s", NumBytesString
>>> ?
>>>
>>
>> Are you asking me? Or the author of the original code?
> 
> Well, the original code used NumBytesString, and your updated version
> does not.
> 
> As per Laszlo's comment - the implementation of
> AsciiStrHexToUint64 means that an arbitrarily long string of leading
> zeroes could be handled by this version that would not previously have
> been handled.
> 
> If that is desired behaviour, then that makes this change a bugfix
> rather than just an API cleanup. Which should be mentioned in the
> commit message. If you do that:
> 
> Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

Yes, I agree it's an improvement if Ard spells out the "added
robustness" in the commit message. (Should not require a repost.)

In this case, we do have a comment in the function:

  // Argument is 8-character ASCII string hex representation of number
  // of bytes that will be sent in the data phase.
  // Response is "DATA" + that same 8-character string.

Honestly, I didn't trust it fully. I didn't verify where the data comes
from, so the comment could be true. But, in all such cases, as a general
principle, I re-format the string representation from the parsed integer
value. It deals nicely with leading "no-op" characters, such as space
and zeros, and it also drops any trailing garbage even if the input is
*not* overlong.

(For example, if the input is "12zzzz", then AsciiStrHexToUint64() will
return 0x12, and ignore the "zzzz" suffix. I think there's value in not
reproducing such trailing garbage.)

I guess I stick to this principle without much thinking, so I didn't ask
for the commit message to be updated. :)

... Extrapolating a bit, I think you might ask for a commit message
update on patch v2 #8 as well :) Because, in addition to the API
replacement there, the patch fixes a separate, genuine overflow as well
(present in the original code). I think mentioning that fact in passing
in the commit message of v2 #8 should suffice.

Thanks!
Laszlo


> 
> /
>     Leif
> 
>>>> +    mTransport->Send (sizeof Response - 1, Response, &mFatalSendErrorEvent);
>>>>
>>>>      mState = ExpectDataState;
>>>>      mBytesReceivedSoFar = 0;
>>>> @@ -257,8 +259,7 @@ AcceptCmd (
>>>>    }
>>>>
>>>>    // Commands aren't null-terminated. Let's get a null-terminated version.
>>>> -  AsciiStrnCpy (Command, Data, Size);
>>>> -  Command[Size] = '\0';
>>>> +  AsciiStrnCpyS (Command, sizeof Command, Data, Size);
>>>>
>>>>    // Parse command
>>>>    if (MATCH_CMD_LITERAL ("getvar", Command)) {
>>>> --
>>>> 2.7.4
>>>>

Re: [PATCH v2 6/9] EmbeddedPkg/Ebl: eliminate deprecated string function calls

[Leif Lindholm]

On Fri, Oct 28, 2016 at 11:44:35AM +0100, Ard Biesheuvel wrote:
> Get rid of calls to unsafe string functions. These are deprecated and may
> be removed in the future.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  EmbeddedPkg/Ebl/Command.c   |  2 +-
>  EmbeddedPkg/Ebl/Dir.c       |  4 ++--
>  EmbeddedPkg/Ebl/EfiDevice.c | 11 ++++++-----
>  EmbeddedPkg/Ebl/Main.c      |  8 ++++----
>  EmbeddedPkg/Ebl/Variable.c  | 17 +++++++++++------
>  5 files changed, 24 insertions(+), 18 deletions(-)
> 
> diff --git a/EmbeddedPkg/Ebl/Command.c b/EmbeddedPkg/Ebl/Command.c
> index e75c6a2e5c32..4bc1f4df0ca0 100644
> --- a/EmbeddedPkg/Ebl/Command.c
> +++ b/EmbeddedPkg/Ebl/Command.c
> @@ -614,7 +614,7 @@ OutputData (
>    UINTN Spaces   = 0;
>    CHAR8 Blanks[80];
>  
> -  AsciiStrCpy (Blanks, mBlanks);
> +  AsciiStrCpyS (Blanks, sizeof Blanks, mBlanks);
>    for (EndAddress = Address + Length; Address < EndAddress; Offset += Line) {
>      AsciiPrint ("%08x: ", Offset);
>      for (Line = 0; (Line < 0x10) && (Address < EndAddress);) {
> diff --git a/EmbeddedPkg/Ebl/Dir.c b/EmbeddedPkg/Ebl/Dir.c
> index 36095b633019..8dd9d48ff6ac 100644
> --- a/EmbeddedPkg/Ebl/Dir.c
> +++ b/EmbeddedPkg/Ebl/Dir.c
> @@ -116,7 +116,7 @@ EblDirCmd (
>      UnicodeFileName[0] = '\0';
>      MatchSubString = &UnicodeFileName[0];
>      if (Argc > 2) {
> -      AsciiStrToUnicodeStr (Argv[2], UnicodeFileName);
> +      AsciiStrToUnicodeStrS (Argv[2], UnicodeFileName, MAX_CMD_LINE);

Actually (more bikeshedding, sorry), as Laszlo just pointed out
ARRAY_SIZE, could we have ARRAY_SIZE(UnicodeFileName) in this function
instead?

Anyway, that's a suggestion, not a demand. Either way:
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

>        if (UnicodeFileName[0] == '*') {
>          // Handle *Name substring matching
>          MatchSubString = &UnicodeFileName[1];
> @@ -231,7 +231,7 @@ EblDirCmd (
>      MatchSubString = NULL;
>      UnicodeFileName[0] = '\0';
>      if (Argc > 2) {
> -      AsciiStrToUnicodeStr (Argv[2], UnicodeFileName);
> +      AsciiStrToUnicodeStrS (Argv[2], UnicodeFileName, MAX_CMD_LINE);
>        if (UnicodeFileName[0] == '*') {
>          MatchSubString = &UnicodeFileName[1];
>        }
> diff --git a/EmbeddedPkg/Ebl/EfiDevice.c b/EmbeddedPkg/Ebl/EfiDevice.c
> index ec9c331b7004..f6969e7b2b05 100644
> --- a/EmbeddedPkg/Ebl/EfiDevice.c
> +++ b/EmbeddedPkg/Ebl/EfiDevice.c
> @@ -343,7 +343,7 @@ EblStartCmd (
>  
>        ImageInfo->LoadOptionsSize = (UINT32)AsciiStrSize (Argv[2]);
>        ImageInfo->LoadOptions     = AllocatePool (ImageInfo->LoadOptionsSize);
> -      AsciiStrCpy (ImageInfo->LoadOptions, Argv[2]);
> +      AsciiStrCpyS (ImageInfo->LoadOptions, ImageInfo->LoadOptionsSize, Argv[2]);
>      }
>  
>      // Transfer control to the EFI image we loaded with LoadImage()
> @@ -741,7 +741,7 @@ EblFileCopyCmd (
>    UINTN         Size;
>    UINTN         Offset;
>    UINTN         Chunk        = FILE_COPY_CHUNK;
> -  UINTN         FileNameLen;
> +  UINTN         FileNameLen, DestFileNameLen;
>    CHAR8*        DestFileName;
>    CHAR8*        SrcFileName;
>    CHAR8*        SrcPtr;
> @@ -786,9 +786,10 @@ EblFileCopyCmd (
>      }
>  
>      // Construct the destination filepath
> -    DestFileName = (CHAR8*)AllocatePool (FileNameLen + AsciiStrLen (SrcFileName) + 1);
> -    AsciiStrCpy (DestFileName, Argv[2]);
> -    AsciiStrCat (DestFileName, SrcFileName);
> +    DestFileNameLen = FileNameLen + AsciiStrLen (SrcFileName) + 1;
> +    DestFileName = (CHAR8*)AllocatePool (DestFileNameLen);
> +    AsciiStrCpyS (DestFileName, DestFileNameLen, Argv[2]);
> +    AsciiStrCatS (DestFileName, DestFileNameLen, SrcFileName);
>    }
>  
>    Source = EfiOpen(Argv[1], EFI_FILE_MODE_READ, 0);
> diff --git a/EmbeddedPkg/Ebl/Main.c b/EmbeddedPkg/Ebl/Main.c
> index 18b2878f69a1..62f559fccfe8 100644
> --- a/EmbeddedPkg/Ebl/Main.c
> +++ b/EmbeddedPkg/Ebl/Main.c
> @@ -88,7 +88,7 @@ SetCmdHistory (
>      }
>  
>      // Copy the new command line into the ring buffer
> -    AsciiStrnCpy(&mCmdHistory[mCmdHistoryStart][0], Cmd, MAX_CMD_LINE);
> +    AsciiStrnCpyS (&mCmdHistory[mCmdHistoryStart][0], MAX_CMD_LINE, Cmd, MAX_CMD_LINE);
>    }
>  
>    // Reset the command history for the next up arrow press
> @@ -432,7 +432,7 @@ GetCmd (
>        }
>        AsciiPrint (History);
>        Index = AsciiStrLen (History);
> -      AsciiStrnCpy (Cmd, History, CmdMaxSize);
> +      AsciiStrnCpyS (Cmd, CmdMaxSize, History, CmdMaxSize);
>      } else {
>        Cmd[Index++] = Char;
>        if (FixedPcdGetBool(PcdEmbeddedShellCharacterEcho) == TRUE) {
> @@ -644,14 +644,14 @@ EdkBootLoaderEntry (
>  
>      Status = gRT->GetVariable(CommandLineVariableName, &VendorGuid, NULL, &CommandLineVariableSize, CommandLineVariable);
>      if (!EFI_ERROR(Status)) {
> -      UnicodeStrToAsciiStr(CommandLineVariable, CmdLine);
> +      UnicodeStrToAsciiStrS (CommandLineVariable, CmdLine, MAX_CMD_LINE);
>      }
>  
>      FreePool(CommandLineVariable);
>    }
>  
>    if (EFI_ERROR(Status)) {
> -    AsciiStrCpy (CmdLine, (CHAR8 *)PcdGetPtr (PcdEmbeddedAutomaticBootCommand));
> +    AsciiStrCpyS (CmdLine, MAX_CMD_LINE, (CHAR8 *)PcdGetPtr (PcdEmbeddedAutomaticBootCommand));
>    }
>  
>    for (;;) {
> diff --git a/EmbeddedPkg/Ebl/Variable.c b/EmbeddedPkg/Ebl/Variable.c
> index f440c48f16dd..92464a6b7133 100644
> --- a/EmbeddedPkg/Ebl/Variable.c
> +++ b/EmbeddedPkg/Ebl/Variable.c
> @@ -29,6 +29,7 @@ EblGetCmd (
>    VOID*       Value;
>    CHAR8*      AsciiVariableName = NULL;
>    CHAR16*     VariableName;
> +  UINTN       VariableNameLen;
>    UINT32      Index;
>  
>    if (Argc == 1) {
> @@ -48,8 +49,9 @@ EblGetCmd (
>      AsciiPrint("Variable name is missing.\n");
>      return Status;
>    } else {
> -    VariableName = AllocatePool((AsciiStrLen (AsciiVariableName) + 1) * sizeof (CHAR16));
> -    AsciiStrToUnicodeStr (AsciiVariableName,VariableName);
> +    VariableNameLen = AsciiStrLen (AsciiVariableName) + 1;
> +    VariableName = AllocatePool (VariableNameLen * sizeof (CHAR16));
> +    AsciiStrToUnicodeStrS (AsciiVariableName, VariableName, VariableNameLen);
>    }
>  
>    // Try to get the variable size.
> @@ -93,6 +95,7 @@ EblSetCmd (
>    CHAR8*        AsciiValue;
>    UINT32        AsciiValueLength;
>    CHAR16*       VariableName;
> +  UINTN         VariableNameLen;
>    UINT32        Index;
>    UINT32        EscapedQuotes = 0;
>    BOOLEAN       Volatile = FALSE;
> @@ -125,8 +128,9 @@ EblSetCmd (
>      //
>  
>      // Convert VariableName into Unicode
> -    VariableName = AllocatePool((AsciiStrLen (AsciiVariableSetting) + 1) * sizeof (CHAR16));
> -    AsciiStrToUnicodeStr (AsciiVariableSetting,VariableName);
> +    VariableNameLen = AsciiStrLen (AsciiVariableSetting) + 1;
> +    VariableName = AllocatePool (VariableNameLen * sizeof (CHAR16));
> +    AsciiStrToUnicodeStrS (AsciiVariableSetting, VariableName, VariableNameLen);
>  
>      Status = gRT->SetVariable (
>                            VariableName,
> @@ -170,8 +174,9 @@ EblSetCmd (
>    }
>  
>    // Convert VariableName into Unicode
> -  VariableName = AllocatePool((AsciiStrLen (AsciiVariableName) + 1) * sizeof (CHAR16));
> -  AsciiStrToUnicodeStr (AsciiVariableName,VariableName);
> +  VariableNameLen = AsciiStrLen (AsciiVariableName) + 1;
> +  VariableName = AllocatePool (VariableNameLen * sizeof (CHAR16));
> +  AsciiStrToUnicodeStrS (AsciiVariableName, VariableName, VariableNameLen);
>  
>    Status = gRT->SetVariable (
>                        VariableName,
> -- 
> 2.7.4
> 

Re: [PATCH v2 7/9] EmbeddedPkg/EfiFileLib: eliminate deprecated string function calls

[Leif Lindholm]

On Fri, Oct 28, 2016 at 11:44:36AM +0100, Ard Biesheuvel wrote:
> Get rid of calls to unsafe string functions. These are deprecated and may
> be removed in the future.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> ---
>  EmbeddedPkg/Library/EfiFileLib/EfiFileLib.c | 42 +++++++++++---------
>  1 file changed, 23 insertions(+), 19 deletions(-)
> 
> diff --git a/EmbeddedPkg/Library/EfiFileLib/EfiFileLib.c b/EmbeddedPkg/Library/EfiFileLib/EfiFileLib.c
> index 4d58c830861c..d3b65aa5a3e0 100644
> --- a/EmbeddedPkg/Library/EfiFileLib/EfiFileLib.c
> +++ b/EmbeddedPkg/Library/EfiFileLib/EfiFileLib.c
> @@ -384,9 +384,9 @@ EblFileDevicePath (
>  
>  
>    if ( *FileName != 0 ) {
> -    AsciiStrToUnicodeStr (FileName, UnicodeFileName);
> +    AsciiStrToUnicodeStrS (FileName, UnicodeFileName, MAX_PATHNAME);

So, I could suggest ARRAY_SIZE again, but since the define here makes
more semantic sense to begin with, it matters even less.

Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

>    } else {
> -    AsciiStrToUnicodeStr ("\\", UnicodeFileName);
> +    AsciiStrToUnicodeStrS ("\\", UnicodeFileName, MAX_PATHNAME);
>    }
>  
>    Size = StrSize (UnicodeFileName);
> @@ -589,7 +589,7 @@ EblFvFileDevicePath (
>            &AuthenticationStatus
>            );
>          if (!EFI_ERROR (Status)) {
> -          UnicodeStrToAsciiStr (Section, AsciiSection);
> +          UnicodeStrToAsciiStrS (Section, AsciiSection, MAX_PATHNAME);
>            if (AsciiStriCmp (FileName, AsciiSection) == 0) {
>              FreePool (Section);
>              break;
> @@ -674,6 +674,7 @@ EfiOpen (
>    CHAR8                     *CwdPlusPathName;
>    UINTN                     Index;
>    EFI_SECTION_TYPE          ModifiedSectionType;
> +  UINTN                     AsciiLength;
>  
>    EblUpdateDeviceLists ();
>  
> @@ -706,7 +707,8 @@ EfiOpen (
>        }
>  
>        // We could add a current working directory concept
> -      CwdPlusPathName = AllocatePool (AsciiStrSize (gCwd) + AsciiStrSize (PathName));
> +      AsciiLength = AsciiStrSize (gCwd) + AsciiStrSize (PathName);
> +      CwdPlusPathName = AllocatePool (AsciiLength);
>        if (CwdPlusPathName == NULL) {
>          return NULL;
>        }
> @@ -723,14 +725,14 @@ EfiOpen (
>            }
>          }
>        } else {
> -        AsciiStrCpy (CwdPlusPathName, gCwd);
> +        AsciiStrCpyS (CwdPlusPathName, AsciiLength, gCwd);
>          StrLen = AsciiStrLen (gCwd);
>          if ((*PathName != '/') && (*PathName != '\\') && (gCwd[StrLen-1] != '/') && (gCwd[StrLen-1] != '\\')) {
> -          AsciiStrCat (CwdPlusPathName, "\\");
> +          AsciiStrCatS (CwdPlusPathName, AsciiLength, "\\");
>          }
>        }
>  
> -      AsciiStrCat (CwdPlusPathName, PathName);
> +      AsciiStrCatS (CwdPlusPathName, AsciiLength, PathName);
>        if (AsciiStrStr (CwdPlusPathName, ":") == NULL) {
>          // Extra error check to make sure we don't recurse and blow stack
>          return NULL;
> @@ -745,7 +747,7 @@ EfiOpen (
>    }
>  
>    File->DeviceName = AllocatePool (StrLen);
> -  AsciiStrCpy (File->DeviceName, PathName);
> +  AsciiStrCpyS (File->DeviceName, StrLen, PathName);
>    File->DeviceName[FileStart - 1] = '\0';
>    File->FileName = &File->DeviceName[FileStart];
>    if (File->FileName[0] == '\0') {
> @@ -1611,7 +1613,7 @@ ExpandPath (
>  {
>    CHAR8   *NewPath;
>    CHAR8   *Work, *Start, *End;
> -  UINTN   StrLen;
> +  UINTN   StrLen, AllocLen;
>    INTN    i;
>  
>    if (Cwd == NULL || Path == NULL) {
> @@ -1625,11 +1627,12 @@ ExpandPath (
>    }
>  
>    StrLen = AsciiStrSize (Path);
> -  NewPath = AllocatePool (AsciiStrSize (Cwd) + StrLen + 1);
> +  AllocLen = AsciiStrSize (Cwd) + StrLen + 1;
> +  NewPath = AllocatePool (AllocLen);
>    if (NewPath == NULL) {
>      return NULL;
>    }
> -  AsciiStrCpy (NewPath, Cwd);
> +  AsciiStrCpyS (NewPath, AllocLen, Cwd);
>  
>    End = Path + StrLen;
>    for (Start = Path ;;) {
> @@ -1640,7 +1643,7 @@ ExpandPath (
>      }
>  
>      // append path prior to ..
> -    AsciiStrnCat (NewPath, Start, Work - Start);
> +    AsciiStrnCatS (NewPath, AllocLen, Start, Work - Start);
>      StrLen = AsciiStrLen (NewPath);
>      for (i = StrLen; i >= 0; i--) {
>        if (NewPath[i] == ':') {
> @@ -1663,7 +1666,7 @@ ExpandPath (
>    }
>  
>    // Handle the path that remains after the ..
> -  AsciiStrnCat (NewPath, Start, End - Start);
> +  AsciiStrnCatS (NewPath, AllocLen, Start, End - Start);
>  
>    return NewPath;
>  }
> @@ -1686,7 +1689,7 @@ EfiSetCwd (
>    )
>  {
>    EFI_OPEN_FILE *File;
> -  UINTN         Len;
> +  UINTN         Len, AllocLen;
>    CHAR8         *Path;
>  
>    if (Cwd == NULL) {
> @@ -1729,17 +1732,18 @@ EfiSetCwd (
>  
>    // Use the info returned from EfiOpen as it can add in CWD if needed. So Cwd could be
>    // relative to the current gCwd or not.
> -  gCwd = AllocatePool (AsciiStrSize (File->DeviceName) + AsciiStrSize (File->FileName) + 10);
> +  AllocLen = AsciiStrSize (File->DeviceName) + AsciiStrSize (File->FileName) + 10;
> +  gCwd = AllocatePool (AllocLen);
>    if (gCwd == NULL) {
>      return EFI_INVALID_PARAMETER;
>    }
>  
> -  AsciiStrCpy (gCwd, File->DeviceName);
> +  AsciiStrCpyS (gCwd, AllocLen, File->DeviceName);
>    if (File->FileName == NULL) {
> -    AsciiStrCat (gCwd, ":\\");
> +    AsciiStrCatS (gCwd, AllocLen, ":\\");
>    } else {
> -    AsciiStrCat (gCwd, ":");
> -    AsciiStrCat (gCwd, File->FileName);
> +    AsciiStrCatS (gCwd, AllocLen, ":");
> +    AsciiStrCatS (gCwd, AllocLen, File->FileName);
>    }
>  
>  
> -- 
> 2.7.4
> 

Re: [PATCH v2 8/9] EmbeddedPkg/MmcDxe: eliminate deprecated string function calls

[Leif Lindholm]

On Fri, Oct 28, 2016 at 11:44:37AM +0100, Ard Biesheuvel wrote:
> Get rid of calls to unsafe string functions. These are deprecated and may
> be removed in the future.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

(I also agree with Laszlo's comment regarding the commit message.)

> ---
>  EmbeddedPkg/Universal/MmcDxe/Diagnostics.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/EmbeddedPkg/Universal/MmcDxe/Diagnostics.c b/EmbeddedPkg/Universal/MmcDxe/Diagnostics.c
> index 783e548d2aed..7d6a5a0dde01 100644
> --- a/EmbeddedPkg/Universal/MmcDxe/Diagnostics.c
> +++ b/EmbeddedPkg/Universal/MmcDxe/Diagnostics.c
> @@ -42,9 +42,9 @@ DiagnosticLog (
>    )
>  {
>    UINTN len = StrLen (Str);
> -  if (len <= mLogRemainChar) {
> +  if (len < mLogRemainChar) {
> +    StrCpyS (mLogBuffer, mLogRemainChar, Str);
>      mLogRemainChar -= len;
> -    StrCpy (mLogBuffer, Str);
>      mLogBuffer += len;
>      return len;
>    } else {
> -- 
> 2.7.4
> 

Re: [PATCH v2 9/9] EmbeddedPkg: enable -DDISABLE_NEW_DEPRECATED_INTERFACES

[Leif Lindholm]

On Fri, Oct 28, 2016 at 11:44:38AM +0100, Ard Biesheuvel wrote:
> Define DISABLE_NEW_DEPRECATED_INTERFACES on the compiler command line by
> default, to prevent deprecated interfaces from being used in core EDK2
> code.
> 
> Bug: https://bugzilla.tianocore.org/show_bug.cgi?id=164
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

> ---
>  EmbeddedPkg/EmbeddedPkg.dsc | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/EmbeddedPkg/EmbeddedPkg.dsc b/EmbeddedPkg/EmbeddedPkg.dsc
> index cc95564f31af..eb7af800f0b2 100644
> --- a/EmbeddedPkg/EmbeddedPkg.dsc
> +++ b/EmbeddedPkg/EmbeddedPkg.dsc
> @@ -4,6 +4,7 @@
>  #
>  # Copyright (c) 2007 - 2015, Intel Corporation. All rights reserved.<BR>
>  # Copyright (c) 2012-2015, ARM Ltd. All rights reserved.<BR>
> +# Copyright (c) 2016, Linaro Ltd. All rights reserved.<BR>
>  #
>  #    This program and the accompanying materials
>  #    are licensed and made available under the terms and conditions of the BSD License
> @@ -227,7 +228,7 @@ [PcdsFixedAtBuild.IPF]
>  
>  [BuildOptions]
>    RVCT:*_*_ARM_PLATFORM_FLAGS == --cpu=7-A.security
> -
> +  *_*_*_CC_FLAGS  = -DDISABLE_NEW_DEPRECATED_INTERFACES
>  
>  ################################################################################
>  #
> -- 
> 2.7.4
> 

Re: [PATCH v2 0/9] EmbeddedPkg: eliminate calls to deprecated functions

[Ard Biesheuvel]

On 28 October 2016 at 11:44, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> This series prepares EmbeddedPkg for building with the preprocess symbol
> DISABLE_NEW_DEPRECATED_INTERFACES defined, by adding missing components
> to EmbeddedPkg (#3), fixing broken code or code that relies on deprecated
> functionality (#1 - #2, #4 - #8), and finally adds
> -DDISABLE_NEW_DEPRECATED_INTERFACES to the CC flags for all build types,
> toolchains and architectures.
>
> v2:
> - incorporated Laszlo's feedback
> - add some R-b's
>
> Again, a *big* thank you to Laszlo for taking the time to review these
> patches. I do apologize for not being as thorough as I could have been,
> resulting in issues in the code that Laszlo spotted in his review.
>
> Bug: https://bugzilla.tianocore.org/show_bug.cgi?id=164
>
> Ard Biesheuvel (9):
>   EmbeddedPkg/AndroidFastbootTransportTcpDxe: remove broken hostname
>     handling
>   EmbeddedPkg: remove unused PrePiHobListPointerLib
>   EmbeddedPkg: add missing modules
>   EmbeddedPkg/GdbDebugAgent: fix VOID* cast of incorrect size
>   EmbeddedPkg/AndroidFastboot: eliminate deprecated string function
>     calls
>   EmbeddedPkg/Ebl: eliminate deprecated string function calls
>   EmbeddedPkg/EfiFileLib: eliminate deprecated string function calls
>   EmbeddedPkg/MmcDxe: eliminate deprecated string function calls
>   EmbeddedPkg: enable -DDISABLE_NEW_DEPRECATED_INTERFACES
>

Pushed, with Leif's last comments regarding ARRAY_SIZE () addressed as well.

Many thanks for the time and effort spent in reviewing this!

-- 
Ard.