* [PATCH v2 1/9] EmbeddedPkg/AndroidFastbootTransportTcpDxe: remove broken hostname handling
2016-10-28 10:44 [PATCH v2 0/9] EmbeddedPkg: eliminate calls to deprecated functions Ard Biesheuvel
@ 2016-10-28 10:44 ` Ard Biesheuvel
2016-10-28 12:40 ` Leif Lindholm
2016-10-28 10:44 ` [PATCH v2 2/9] EmbeddedPkg: remove unused PrePiHobListPointerLib Ard Biesheuvel
` (8 subsequent siblings)
9 siblings, 1 reply; 28+ messages in thread
From: Ard Biesheuvel @ 2016-10-28 10:44 UTC (permalink / raw)
To: edk2-devel, leif.lindholm; +Cc: lersek, ryan.harkin, 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
^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH v2 1/9] EmbeddedPkg/AndroidFastbootTransportTcpDxe: remove broken hostname handling
2016-10-28 10:44 ` [PATCH v2 1/9] EmbeddedPkg/AndroidFastbootTransportTcpDxe: remove broken hostname handling Ard Biesheuvel
@ 2016-10-28 12:40 ` Leif Lindholm
0 siblings, 0 replies; 28+ messages in thread
From: Leif Lindholm @ 2016-10-28 12:40 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: edk2-devel, lersek, ryan.harkin
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
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v2 2/9] EmbeddedPkg: remove unused PrePiHobListPointerLib
2016-10-28 10:44 [PATCH v2 0/9] EmbeddedPkg: eliminate calls to deprecated functions Ard Biesheuvel
2016-10-28 10:44 ` [PATCH v2 1/9] EmbeddedPkg/AndroidFastbootTransportTcpDxe: remove broken hostname handling Ard Biesheuvel
@ 2016-10-28 10:44 ` Ard Biesheuvel
2016-10-28 12:40 ` Leif Lindholm
2016-10-28 10:44 ` [PATCH v2 3/9] EmbeddedPkg: add missing modules Ard Biesheuvel
` (7 subsequent siblings)
9 siblings, 1 reply; 28+ messages in thread
From: Ard Biesheuvel @ 2016-10-28 10:44 UTC (permalink / raw)
To: edk2-devel, leif.lindholm; +Cc: lersek, ryan.harkin, 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
^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH v2 2/9] EmbeddedPkg: remove unused PrePiHobListPointerLib
2016-10-28 10:44 ` [PATCH v2 2/9] EmbeddedPkg: remove unused PrePiHobListPointerLib Ard Biesheuvel
@ 2016-10-28 12:40 ` Leif Lindholm
0 siblings, 0 replies; 28+ messages in thread
From: Leif Lindholm @ 2016-10-28 12:40 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: edk2-devel, lersek, ryan.harkin
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
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v2 3/9] EmbeddedPkg: add missing modules
2016-10-28 10:44 [PATCH v2 0/9] EmbeddedPkg: eliminate calls to deprecated functions Ard Biesheuvel
2016-10-28 10:44 ` [PATCH v2 1/9] EmbeddedPkg/AndroidFastbootTransportTcpDxe: remove broken hostname handling Ard Biesheuvel
2016-10-28 10:44 ` [PATCH v2 2/9] EmbeddedPkg: remove unused PrePiHobListPointerLib Ard Biesheuvel
@ 2016-10-28 10:44 ` Ard Biesheuvel
2016-10-28 12:41 ` Leif Lindholm
2016-10-28 10:44 ` [PATCH v2 4/9] EmbeddedPkg/GdbDebugAgent: fix VOID* cast of incorrect size Ard Biesheuvel
` (6 subsequent siblings)
9 siblings, 1 reply; 28+ messages in thread
From: Ard Biesheuvel @ 2016-10-28 10:44 UTC (permalink / raw)
To: edk2-devel, leif.lindholm; +Cc: lersek, ryan.harkin, 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
^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH v2 3/9] EmbeddedPkg: add missing modules
2016-10-28 10:44 ` [PATCH v2 3/9] EmbeddedPkg: add missing modules Ard Biesheuvel
@ 2016-10-28 12:41 ` Leif Lindholm
0 siblings, 0 replies; 28+ messages in thread
From: Leif Lindholm @ 2016-10-28 12:41 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: edk2-devel, lersek, ryan.harkin
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
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v2 4/9] EmbeddedPkg/GdbDebugAgent: fix VOID* cast of incorrect size
2016-10-28 10:44 [PATCH v2 0/9] EmbeddedPkg: eliminate calls to deprecated functions Ard Biesheuvel
` (2 preceding siblings ...)
2016-10-28 10:44 ` [PATCH v2 3/9] EmbeddedPkg: add missing modules Ard Biesheuvel
@ 2016-10-28 10:44 ` Ard Biesheuvel
2016-10-28 12:47 ` Leif Lindholm
2016-10-28 10:44 ` [PATCH v2 5/9] EmbeddedPkg/AndroidFastboot: eliminate deprecated string function calls Ard Biesheuvel
` (5 subsequent siblings)
9 siblings, 1 reply; 28+ messages in thread
From: Ard Biesheuvel @ 2016-10-28 10:44 UTC (permalink / raw)
To: edk2-devel, leif.lindholm; +Cc: lersek, ryan.harkin, 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
^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH v2 4/9] EmbeddedPkg/GdbDebugAgent: fix VOID* cast of incorrect size
2016-10-28 10:44 ` [PATCH v2 4/9] EmbeddedPkg/GdbDebugAgent: fix VOID* cast of incorrect size Ard Biesheuvel
@ 2016-10-28 12:47 ` Leif Lindholm
0 siblings, 0 replies; 28+ messages in thread
From: Leif Lindholm @ 2016-10-28 12:47 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: edk2-devel, lersek, ryan.harkin
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>
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v2 5/9] EmbeddedPkg/AndroidFastboot: eliminate deprecated string function calls
2016-10-28 10:44 [PATCH v2 0/9] EmbeddedPkg: eliminate calls to deprecated functions Ard Biesheuvel
` (3 preceding siblings ...)
2016-10-28 10:44 ` [PATCH v2 4/9] EmbeddedPkg/GdbDebugAgent: fix VOID* cast of incorrect size Ard Biesheuvel
@ 2016-10-28 10:44 ` Ard Biesheuvel
2016-10-28 13:18 ` Laszlo Ersek
2016-10-28 13:36 ` Leif Lindholm
2016-10-28 10:44 ` [PATCH v2 6/9] EmbeddedPkg/Ebl: " Ard Biesheuvel
` (4 subsequent siblings)
9 siblings, 2 replies; 28+ messages in thread
From: Ard Biesheuvel @ 2016-10-28 10:44 UTC (permalink / raw)
To: edk2-devel, leif.lindholm; +Cc: lersek, ryan.harkin, 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
^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH v2 5/9] EmbeddedPkg/AndroidFastboot: eliminate deprecated string function calls
2016-10-28 10:44 ` [PATCH v2 5/9] EmbeddedPkg/AndroidFastboot: eliminate deprecated string function calls Ard Biesheuvel
@ 2016-10-28 13:18 ` Laszlo Ersek
2016-10-28 13:36 ` Leif Lindholm
1 sibling, 0 replies; 28+ messages in thread
From: Laszlo Ersek @ 2016-10-28 13:18 UTC (permalink / raw)
To: Ard Biesheuvel, edk2-devel, leif.lindholm; +Cc: ryan.harkin
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>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 5/9] EmbeddedPkg/AndroidFastboot: eliminate deprecated string function calls
2016-10-28 10:44 ` [PATCH v2 5/9] EmbeddedPkg/AndroidFastboot: eliminate deprecated string function calls Ard Biesheuvel
2016-10-28 13:18 ` Laszlo Ersek
@ 2016-10-28 13:36 ` Leif Lindholm
2016-10-28 13:40 ` Ard Biesheuvel
2016-10-28 13:41 ` Laszlo Ersek
1 sibling, 2 replies; 28+ messages in thread
From: Leif Lindholm @ 2016-10-28 13:36 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: edk2-devel, lersek, ryan.harkin
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
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 5/9] EmbeddedPkg/AndroidFastboot: eliminate deprecated string function calls
2016-10-28 13:36 ` Leif Lindholm
@ 2016-10-28 13:40 ` Ard Biesheuvel
2016-10-28 13:52 ` Leif Lindholm
2016-10-28 13:41 ` Laszlo Ersek
1 sibling, 1 reply; 28+ messages in thread
From: Ard Biesheuvel @ 2016-10-28 13:40 UTC (permalink / raw)
To: Leif Lindholm; +Cc: edk2-devel-01, Laszlo Ersek, Ryan Harkin
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
>>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 5/9] EmbeddedPkg/AndroidFastboot: eliminate deprecated string function calls
2016-10-28 13:40 ` Ard Biesheuvel
@ 2016-10-28 13:52 ` Leif Lindholm
2016-10-28 14:04 ` Ard Biesheuvel
2016-10-28 14:05 ` Laszlo Ersek
0 siblings, 2 replies; 28+ messages in thread
From: Leif Lindholm @ 2016-10-28 13:52 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: edk2-devel-01, Laszlo Ersek, Ryan Harkin
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
> >>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 5/9] EmbeddedPkg/AndroidFastboot: eliminate deprecated string function calls
2016-10-28 13:52 ` Leif Lindholm
@ 2016-10-28 14:04 ` Ard Biesheuvel
2016-10-28 14:05 ` Laszlo Ersek
1 sibling, 0 replies; 28+ messages in thread
From: Ard Biesheuvel @ 2016-10-28 14:04 UTC (permalink / raw)
To: Leif Lindholm; +Cc: edk2-devel-01, Laszlo Ersek, Ryan Harkin
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.
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 5/9] EmbeddedPkg/AndroidFastboot: eliminate deprecated string function calls
2016-10-28 13:52 ` Leif Lindholm
2016-10-28 14:04 ` Ard Biesheuvel
@ 2016-10-28 14:05 ` Laszlo Ersek
1 sibling, 0 replies; 28+ messages in thread
From: Laszlo Ersek @ 2016-10-28 14:05 UTC (permalink / raw)
To: Leif Lindholm, Ard Biesheuvel; +Cc: edk2-devel-01, Ryan Harkin
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
>>>>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 5/9] EmbeddedPkg/AndroidFastboot: eliminate deprecated string function calls
2016-10-28 13:36 ` Leif Lindholm
2016-10-28 13:40 ` Ard Biesheuvel
@ 2016-10-28 13:41 ` Laszlo Ersek
1 sibling, 0 replies; 28+ messages in thread
From: Laszlo Ersek @ 2016-10-28 13:41 UTC (permalink / raw)
To: Leif Lindholm, Ard Biesheuvel; +Cc: edk2-devel, ryan.harkin
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
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v2 6/9] EmbeddedPkg/Ebl: eliminate deprecated string function calls
2016-10-28 10:44 [PATCH v2 0/9] EmbeddedPkg: eliminate calls to deprecated functions Ard Biesheuvel
` (4 preceding siblings ...)
2016-10-28 10:44 ` [PATCH v2 5/9] EmbeddedPkg/AndroidFastboot: eliminate deprecated string function calls Ard Biesheuvel
@ 2016-10-28 10:44 ` Ard Biesheuvel
2016-10-28 13:31 ` Laszlo Ersek
2016-10-28 14:31 ` Leif Lindholm
2016-10-28 10:44 ` [PATCH v2 7/9] EmbeddedPkg/EfiFileLib: " Ard Biesheuvel
` (3 subsequent siblings)
9 siblings, 2 replies; 28+ messages in thread
From: Ard Biesheuvel @ 2016-10-28 10:44 UTC (permalink / raw)
To: edk2-devel, leif.lindholm; +Cc: lersek, ryan.harkin, 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
^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH v2 6/9] EmbeddedPkg/Ebl: eliminate deprecated string function calls
2016-10-28 10:44 ` [PATCH v2 6/9] EmbeddedPkg/Ebl: " Ard Biesheuvel
@ 2016-10-28 13:31 ` Laszlo Ersek
2016-10-28 14:31 ` Leif Lindholm
1 sibling, 0 replies; 28+ messages in thread
From: Laszlo Ersek @ 2016-10-28 13:31 UTC (permalink / raw)
To: Ard Biesheuvel, edk2-devel, leif.lindholm; +Cc: ryan.harkin
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>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 6/9] EmbeddedPkg/Ebl: eliminate deprecated string function calls
2016-10-28 10:44 ` [PATCH v2 6/9] EmbeddedPkg/Ebl: " Ard Biesheuvel
2016-10-28 13:31 ` Laszlo Ersek
@ 2016-10-28 14:31 ` Leif Lindholm
1 sibling, 0 replies; 28+ messages in thread
From: Leif Lindholm @ 2016-10-28 14:31 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: edk2-devel, lersek, ryan.harkin
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
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v2 7/9] EmbeddedPkg/EfiFileLib: eliminate deprecated string function calls
2016-10-28 10:44 [PATCH v2 0/9] EmbeddedPkg: eliminate calls to deprecated functions Ard Biesheuvel
` (5 preceding siblings ...)
2016-10-28 10:44 ` [PATCH v2 6/9] EmbeddedPkg/Ebl: " Ard Biesheuvel
@ 2016-10-28 10:44 ` Ard Biesheuvel
2016-10-28 14:37 ` Leif Lindholm
2016-10-28 10:44 ` [PATCH v2 8/9] EmbeddedPkg/MmcDxe: " Ard Biesheuvel
` (2 subsequent siblings)
9 siblings, 1 reply; 28+ messages in thread
From: Ard Biesheuvel @ 2016-10-28 10:44 UTC (permalink / raw)
To: edk2-devel, leif.lindholm; +Cc: lersek, ryan.harkin, 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
^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH v2 7/9] EmbeddedPkg/EfiFileLib: eliminate deprecated string function calls
2016-10-28 10:44 ` [PATCH v2 7/9] EmbeddedPkg/EfiFileLib: " Ard Biesheuvel
@ 2016-10-28 14:37 ` Leif Lindholm
0 siblings, 0 replies; 28+ messages in thread
From: Leif Lindholm @ 2016-10-28 14:37 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: edk2-devel, lersek, ryan.harkin
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
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v2 8/9] EmbeddedPkg/MmcDxe: eliminate deprecated string function calls
2016-10-28 10:44 [PATCH v2 0/9] EmbeddedPkg: eliminate calls to deprecated functions Ard Biesheuvel
` (6 preceding siblings ...)
2016-10-28 10:44 ` [PATCH v2 7/9] EmbeddedPkg/EfiFileLib: " Ard Biesheuvel
@ 2016-10-28 10:44 ` Ard Biesheuvel
2016-10-28 13:40 ` Laszlo Ersek
2016-10-28 14:39 ` Leif Lindholm
2016-10-28 10:44 ` [PATCH v2 9/9] EmbeddedPkg: enable -DDISABLE_NEW_DEPRECATED_INTERFACES Ard Biesheuvel
2016-10-28 15:16 ` [PATCH v2 0/9] EmbeddedPkg: eliminate calls to deprecated functions Ard Biesheuvel
9 siblings, 2 replies; 28+ messages in thread
From: Ard Biesheuvel @ 2016-10-28 10:44 UTC (permalink / raw)
To: edk2-devel, leif.lindholm; +Cc: lersek, ryan.harkin, 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
^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH v2 8/9] EmbeddedPkg/MmcDxe: eliminate deprecated string function calls
2016-10-28 10:44 ` [PATCH v2 8/9] EmbeddedPkg/MmcDxe: " Ard Biesheuvel
@ 2016-10-28 13:40 ` Laszlo Ersek
2016-10-28 14:39 ` Leif Lindholm
1 sibling, 0 replies; 28+ messages in thread
From: Laszlo Ersek @ 2016-10-28 13:40 UTC (permalink / raw)
To: Ard Biesheuvel, edk2-devel, leif.lindholm; +Cc: ryan.harkin
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>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 8/9] EmbeddedPkg/MmcDxe: eliminate deprecated string function calls
2016-10-28 10:44 ` [PATCH v2 8/9] EmbeddedPkg/MmcDxe: " Ard Biesheuvel
2016-10-28 13:40 ` Laszlo Ersek
@ 2016-10-28 14:39 ` Leif Lindholm
1 sibling, 0 replies; 28+ messages in thread
From: Leif Lindholm @ 2016-10-28 14:39 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: edk2-devel, lersek, ryan.harkin
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
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v2 9/9] EmbeddedPkg: enable -DDISABLE_NEW_DEPRECATED_INTERFACES
2016-10-28 10:44 [PATCH v2 0/9] EmbeddedPkg: eliminate calls to deprecated functions Ard Biesheuvel
` (7 preceding siblings ...)
2016-10-28 10:44 ` [PATCH v2 8/9] EmbeddedPkg/MmcDxe: " Ard Biesheuvel
@ 2016-10-28 10:44 ` Ard Biesheuvel
2016-10-28 14:40 ` Leif Lindholm
2016-10-28 15:16 ` [PATCH v2 0/9] EmbeddedPkg: eliminate calls to deprecated functions Ard Biesheuvel
9 siblings, 1 reply; 28+ messages in thread
From: Ard Biesheuvel @ 2016-10-28 10:44 UTC (permalink / raw)
To: edk2-devel, leif.lindholm; +Cc: lersek, ryan.harkin, 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
^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH v2 9/9] EmbeddedPkg: enable -DDISABLE_NEW_DEPRECATED_INTERFACES
2016-10-28 10:44 ` [PATCH v2 9/9] EmbeddedPkg: enable -DDISABLE_NEW_DEPRECATED_INTERFACES Ard Biesheuvel
@ 2016-10-28 14:40 ` Leif Lindholm
0 siblings, 0 replies; 28+ messages in thread
From: Leif Lindholm @ 2016-10-28 14:40 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: edk2-devel, lersek, ryan.harkin
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
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 0/9] EmbeddedPkg: eliminate calls to deprecated functions
2016-10-28 10:44 [PATCH v2 0/9] EmbeddedPkg: eliminate calls to deprecated functions Ard Biesheuvel
` (8 preceding siblings ...)
2016-10-28 10:44 ` [PATCH v2 9/9] EmbeddedPkg: enable -DDISABLE_NEW_DEPRECATED_INTERFACES Ard Biesheuvel
@ 2016-10-28 15:16 ` Ard Biesheuvel
9 siblings, 0 replies; 28+ messages in thread
From: Ard Biesheuvel @ 2016-10-28 15:16 UTC (permalink / raw)
To: edk2-devel-01, Leif Lindholm; +Cc: Laszlo Ersek, Ryan Harkin, 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.
^ permalink raw reply [flat|nested] 28+ messages in thread