* [PATCH 0/6] ArmPkg: eliminate calls to deprecated functions
@ 2016-10-24 16:06 Ard Biesheuvel
2016-10-24 16:06 ` [PATCH 1/6] ArmPkg: add missing components Ard Biesheuvel
` (6 more replies)
0 siblings, 7 replies; 17+ messages in thread
From: Ard Biesheuvel @ 2016-10-24 16:06 UTC (permalink / raw)
To: edk2-devel, leif.lindholm, lersek; +Cc: Ard Biesheuvel
This series prepares ArmPkg for building with the preprocess symbol
DISABLE_NEW_DEPRECATED_INTERFACES defined, by adding missing components
to ArmPkg (#1), fixing broken code or code that relies on deprecated
functionality (#2 - #5), and finally adds -DDISABLE_NEW_DEPRECATED_INTERFACES
to the CC flags for all build types, toolchains and architectures.
Bug: https://bugzilla.tianocore.org/show_bug.cgi?id=164
Ard Biesheuvel (6):
ArmPkg: add missing components
ArmPkg/ArmCortexA9Lib RVCT: remove incompatible GCC include
ArmPkg/LinuxLoader: eliminate calls to deprecated string functions
ArmPkg/SemihostFs: eliminate calls to deprecated string functions
ArmPkg/BdsLib: eliminate calls to deprecated string functions
ArmPkg: enable -DDISABLE_NEW_DEPRECATED_INTERFACES
ArmPkg/Application/LinuxLoader/Arm/LinuxAtag.c | 2 +-
ArmPkg/Application/LinuxLoader/LinuxLoader.c | 6 ++++--
ArmPkg/ArmPkg.dsc | 13 +++++++++++++
ArmPkg/Drivers/ArmCpuLib/ArmCortexA9Lib/ArmCortexA9Helper.asm | 4 ----
ArmPkg/Filesystem/SemihostFs/Arm/SemihostFs.c | 20 ++++++++++++--------
ArmPkg/Library/BdsLib/BdsFilePath.c | 8 +++++---
6 files changed, 35 insertions(+), 18 deletions(-)
--
2.7.4
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 1/6] ArmPkg: add missing components
2016-10-24 16:06 [PATCH 0/6] ArmPkg: eliminate calls to deprecated functions Ard Biesheuvel
@ 2016-10-24 16:06 ` Ard Biesheuvel
2016-10-25 12:51 ` Leif Lindholm
2016-10-24 16:06 ` [PATCH 2/6] ArmPkg/ArmCortexA9Lib RVCT: remove incompatible GCC include Ard Biesheuvel
` (5 subsequent siblings)
6 siblings, 1 reply; 17+ messages in thread
From: Ard Biesheuvel @ 2016-10-24 16:06 UTC (permalink / raw)
To: edk2-devel, leif.lindholm, lersek; +Cc: Ard Biesheuvel
ArmPkg.dsc was a bit out of date, and some modules added over the past
years had not been added to its [Components] section yet.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
ArmPkg/ArmPkg.dsc | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/ArmPkg/ArmPkg.dsc b/ArmPkg/ArmPkg.dsc
index 6a8ff7e621d7..9bb1263a7f96 100644
--- a/ArmPkg/ArmPkg.dsc
+++ b/ArmPkg/ArmPkg.dsc
@@ -143,6 +143,16 @@ [Components.common]
ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
+ ArmPkg/Drivers/ArmPciCpuIo2Dxe/ArmPciCpuIo2Dxe.inf
+ ArmPkg/Library/ArmArchTimerLib/ArmArchTimerLib.inf
+ ArmPkg/Library/ArmGicArchLib/ArmGicArchLib.inf
+ ArmPkg/Library/ArmGicArchSecLib/ArmGicArchSecLib.inf
+ ArmPkg/Library/ArmLib/ArmBaseLib.inf
+ ArmPkg/Library/ArmSoftFloatLib/ArmSoftFloatLib.inf
+ ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerLibBase.inf
+ ArmPkg/Library/PeiServicesTablePointerLib/PeiServicesTablePointerLib.inf
+ ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
+
[Components.ARM]
ArmPkg/Drivers/ArmCpuLib/ArmCortexA8Lib/ArmCortexA8Lib.inf
ArmPkg/Drivers/ArmCpuLib/ArmCortexA9Lib/ArmCortexA9Lib.inf
@@ -151,3 +161,4 @@ [Components.ARM]
[Components.AARCH64]
ArmPkg/Drivers/ArmCpuLib/ArmCortexAEMv8Lib/ArmCortexAEMv8Lib.inf
ArmPkg/Drivers/ArmCpuLib/ArmCortexA5xLib/ArmCortexA5xLib.inf
+ ArmPkg/Library/ArmMmuLib/ArmMmuPeiLib.inf
--
2.7.4
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 2/6] ArmPkg/ArmCortexA9Lib RVCT: remove incompatible GCC include
2016-10-24 16:06 [PATCH 0/6] ArmPkg: eliminate calls to deprecated functions Ard Biesheuvel
2016-10-24 16:06 ` [PATCH 1/6] ArmPkg: add missing components Ard Biesheuvel
@ 2016-10-24 16:06 ` Ard Biesheuvel
2016-10-25 12:53 ` Leif Lindholm
2016-10-24 16:06 ` [PATCH 3/6] ArmPkg/LinuxLoader: eliminate calls to deprecated string functions Ard Biesheuvel
` (4 subsequent siblings)
6 siblings, 1 reply; 17+ messages in thread
From: Ard Biesheuvel @ 2016-10-24 16:06 UTC (permalink / raw)
To: edk2-devel, leif.lindholm, lersek; +Cc: Ard Biesheuvel
Drop the include of AsmMacroIoLib.h, which contains GCC preprocessor macros
that RVCT does not use or require, given it has its own AsmMacroIoLib.inc
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
ArmPkg/Drivers/ArmCpuLib/ArmCortexA9Lib/ArmCortexA9Helper.asm | 4 ----
1 file changed, 4 deletions(-)
diff --git a/ArmPkg/Drivers/ArmCpuLib/ArmCortexA9Lib/ArmCortexA9Helper.asm b/ArmPkg/Drivers/ArmCpuLib/ArmCortexA9Lib/ArmCortexA9Helper.asm
index 1417c9a78204..882f25b780cd 100644
--- a/ArmPkg/Drivers/ArmCpuLib/ArmCortexA9Lib/ArmCortexA9Helper.asm
+++ b/ArmPkg/Drivers/ArmCpuLib/ArmCortexA9Lib/ArmCortexA9Helper.asm
@@ -11,10 +11,6 @@
//
//
-#include <AsmMacroIoLib.h>
-#include <Library/ArmCpuLib.h>
-#include <Chipset/ArmCortexA9.h>
-
INCLUDE AsmMacroExport.inc
INCLUDE AsmMacroIoLib.inc
--
2.7.4
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 3/6] ArmPkg/LinuxLoader: eliminate calls to deprecated string functions
2016-10-24 16:06 [PATCH 0/6] ArmPkg: eliminate calls to deprecated functions Ard Biesheuvel
2016-10-24 16:06 ` [PATCH 1/6] ArmPkg: add missing components Ard Biesheuvel
2016-10-24 16:06 ` [PATCH 2/6] ArmPkg/ArmCortexA9Lib RVCT: remove incompatible GCC include Ard Biesheuvel
@ 2016-10-24 16:06 ` Ard Biesheuvel
2016-10-25 10:53 ` Laszlo Ersek
2016-10-24 16:06 ` [PATCH 4/6] ArmPkg/SemihostFs: " Ard Biesheuvel
` (3 subsequent siblings)
6 siblings, 1 reply; 17+ messages in thread
From: Ard Biesheuvel @ 2016-10-24 16:06 UTC (permalink / raw)
To: edk2-devel, leif.lindholm, lersek; +Cc: Ard Biesheuvel
Remove calls to deprecated string functions like AsciiStrCpy() and
UnicodeStrToAsciiStr()
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
ArmPkg/Application/LinuxLoader/Arm/LinuxAtag.c | 2 +-
ArmPkg/Application/LinuxLoader/LinuxLoader.c | 6 ++++--
2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/ArmPkg/Application/LinuxLoader/Arm/LinuxAtag.c b/ArmPkg/Application/LinuxLoader/Arm/LinuxAtag.c
index fd7ee9c8624d..0b3e2489c758 100644
--- a/ArmPkg/Application/LinuxLoader/Arm/LinuxAtag.c
+++ b/ArmPkg/Application/LinuxLoader/Arm/LinuxAtag.c
@@ -72,7 +72,7 @@ SetupCmdlineTag (
mLinuxKernelCurrentAtag->header.type = ATAG_CMDLINE;
/* place CommandLine into tag */
- AsciiStrCpy (mLinuxKernelCurrentAtag->body.cmdline_tag.cmdline, CmdLine);
+ AsciiStrCpyS (mLinuxKernelCurrentAtag->body.cmdline_tag.cmdline, LineLength, CmdLine);
// move pointer to next tag
mLinuxKernelCurrentAtag = next_tag_address (mLinuxKernelCurrentAtag);
diff --git a/ArmPkg/Application/LinuxLoader/LinuxLoader.c b/ArmPkg/Application/LinuxLoader/LinuxLoader.c
index 70b960b66f0e..76697c3a8c9d 100644
--- a/ArmPkg/Application/LinuxLoader/LinuxLoader.c
+++ b/ArmPkg/Application/LinuxLoader/LinuxLoader.c
@@ -61,6 +61,7 @@ LinuxLoaderEntryPoint (
LIST_ENTRY *ResourceLink;
SYSTEM_MEMORY_RESOURCE *Resource;
EFI_PHYSICAL_ADDRESS SystemMemoryBase;
+ UINTN Length;
Status = gBS->LocateProtocol (
&gEfiDevicePathFromTextProtocolGuid,
@@ -182,12 +183,13 @@ LinuxLoaderEntryPoint (
}
if (LinuxCommandLine != NULL) {
- AsciiLinuxCommandLine = AllocatePool ((StrLen (LinuxCommandLine) + 1) * sizeof (CHAR8));
+ Length = StrLen (LinuxCommandLine) + 1;
+ AsciiLinuxCommandLine = AllocatePool (Length);
if (AsciiLinuxCommandLine == NULL) {
Status = EFI_OUT_OF_RESOURCES;
goto Error;
}
- UnicodeStrToAsciiStr (LinuxCommandLine, AsciiLinuxCommandLine);
+ UnicodeStrToAsciiStrS (LinuxCommandLine, AsciiLinuxCommandLine, Length);
}
//
--
2.7.4
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 4/6] ArmPkg/SemihostFs: eliminate calls to deprecated string functions
2016-10-24 16:06 [PATCH 0/6] ArmPkg: eliminate calls to deprecated functions Ard Biesheuvel
` (2 preceding siblings ...)
2016-10-24 16:06 ` [PATCH 3/6] ArmPkg/LinuxLoader: eliminate calls to deprecated string functions Ard Biesheuvel
@ 2016-10-24 16:06 ` Ard Biesheuvel
2016-10-25 11:11 ` Laszlo Ersek
2016-10-24 16:06 ` [PATCH 5/6] ArmPkg/BdsLib: " Ard Biesheuvel
` (2 subsequent siblings)
6 siblings, 1 reply; 17+ messages in thread
From: Ard Biesheuvel @ 2016-10-24 16:06 UTC (permalink / raw)
To: edk2-devel, leif.lindholm, lersek; +Cc: Ard Biesheuvel
Remove calls to deprecated string functions like AsciiStrCpy() and
UnicodeStrToAsciiStr()
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
ArmPkg/Filesystem/SemihostFs/Arm/SemihostFs.c | 20 ++++++++++++--------
1 file changed, 12 insertions(+), 8 deletions(-)
diff --git a/ArmPkg/Filesystem/SemihostFs/Arm/SemihostFs.c b/ArmPkg/Filesystem/SemihostFs/Arm/SemihostFs.c
index 6efdad9ebcce..e79b5cc5cf39 100644
--- a/ArmPkg/Filesystem/SemihostFs/Arm/SemihostFs.c
+++ b/ArmPkg/Filesystem/SemihostFs/Arm/SemihostFs.c
@@ -207,11 +207,12 @@ FileOpen (
return EFI_WRITE_PROTECTED;
}
- AsciiFileName = AllocatePool (StrLen (FileName) + 1);
+ Length = StrLen (FileName) + 1;
+ AsciiFileName = AllocatePool (Length);
if (AsciiFileName == NULL) {
return EFI_OUT_OF_RESOURCES;
}
- UnicodeStrToAsciiStr (FileName, AsciiFileName);
+ UnicodeStrToAsciiStrS (FileName, AsciiFileName, Length);
// Opening '/', '\', '.', or the NULL pathname is trying to open the root directory
if ((AsciiStrCmp (AsciiFileName, "\\") == 0) ||
@@ -463,7 +464,7 @@ FileDelete (
NameSize = AsciiStrLen (Fcb->FileName);
FileName = AllocatePool (NameSize + 1);
- AsciiStrCpy (FileName, Fcb->FileName);
+ AsciiStrCpyS (FileName, NameSize + 1, Fcb->FileName);
// Close the file if it's open. Disregard return status,
// since it might give an error if the file isn't open.
@@ -828,8 +829,10 @@ GetFilesystemInfo (
EFI_FILE_SYSTEM_INFO *Info;
EFI_STATUS Status;
UINTN ResultSize;
+ UINTN StringSize;
- ResultSize = SIZE_OF_EFI_FILE_SYSTEM_INFO + StrSize (mSemihostFsLabel);
+ StringSize = StrSize (mSemihostFsLabel);
+ ResultSize = SIZE_OF_EFI_FILE_SYSTEM_INFO + StringSize;
if (*BufferSize >= ResultSize) {
ZeroMem (Buffer, ResultSize);
@@ -843,7 +846,7 @@ GetFilesystemInfo (
Info->FreeSpace = 0;
Info->BlockSize = 0;
- StrCpy (Info->VolumeLabel, mSemihostFsLabel);
+ StrCpyS (Info->VolumeLabel, StringSize, mSemihostFsLabel);
} else {
Status = EFI_BUFFER_TOO_SMALL;
}
@@ -903,7 +906,7 @@ FileGetInfo (
ResultSize = StrSize (mSemihostFsLabel);
if (*BufferSize >= ResultSize) {
- StrCpy (Buffer, mSemihostFsLabel);
+ StrCpyS (Buffer, *BufferSize, mSemihostFsLabel);
Status = EFI_SUCCESS;
} else {
Status = EFI_BUFFER_TOO_SMALL;
@@ -963,11 +966,12 @@ SetFileInfo (
return EFI_ACCESS_DENIED;
}
- AsciiFileName = AllocatePool (StrLen (Info->FileName) + 1);
+ Length = StrLen (Info->FileName) + 1;
+ AsciiFileName = AllocatePool (Length);
if (AsciiFileName == NULL) {
return EFI_OUT_OF_RESOURCES;
}
- UnicodeStrToAsciiStr (Info->FileName, AsciiFileName);
+ UnicodeStrToAsciiStrS (Info->FileName, AsciiFileName, Length);
FileSizeIsDifferent = (Info->FileSize != Fcb->Info.FileSize);
FileNameIsDifferent = (AsciiStrCmp (AsciiFileName, Fcb->FileName) != 0);
--
2.7.4
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 5/6] ArmPkg/BdsLib: eliminate calls to deprecated string functions
2016-10-24 16:06 [PATCH 0/6] ArmPkg: eliminate calls to deprecated functions Ard Biesheuvel
` (3 preceding siblings ...)
2016-10-24 16:06 ` [PATCH 4/6] ArmPkg/SemihostFs: " Ard Biesheuvel
@ 2016-10-24 16:06 ` Ard Biesheuvel
2016-10-25 11:16 ` Laszlo Ersek
2016-10-24 16:06 ` [PATCH 6/6] ArmPkg: enable -DDISABLE_NEW_DEPRECATED_INTERFACES Ard Biesheuvel
2016-10-25 11:32 ` [PATCH 0/6] ArmPkg: eliminate calls to deprecated functions Ryan Harkin
6 siblings, 1 reply; 17+ messages in thread
From: Ard Biesheuvel @ 2016-10-24 16:06 UTC (permalink / raw)
To: edk2-devel, leif.lindholm, lersek; +Cc: Ard Biesheuvel
Remove calls to deprecated string functions like AsciiStrCpy() and
UnicodeStrToAsciiStr()
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
ArmPkg/Library/BdsLib/BdsFilePath.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/ArmPkg/Library/BdsLib/BdsFilePath.c b/ArmPkg/Library/BdsLib/BdsFilePath.c
index aefeaed4ffb5..351a8bd8edf4 100644
--- a/ArmPkg/Library/BdsLib/BdsFilePath.c
+++ b/ArmPkg/Library/BdsLib/BdsFilePath.c
@@ -964,7 +964,7 @@ Mtftp4CheckPacket (
Step = (Context->DownloadedNbOfBytes * TFTP_PROGRESS_SLIDER_STEPS) / Context->FileSize;
if (Step > LastStep) {
Print (mTftpProgressDelete);
- StrCpy (Progress, mTftpProgressFrame);
+ StrCpyS (Progress, sizeof (Progress), mTftpProgressFrame);
for (Index = 1; Index < Step; Index++) {
Progress[Index] = L'=';
}
@@ -1044,6 +1044,7 @@ BdsTftpLoadImage (
UINT64 FileSize;
UINT64 TftpBufferSize;
BDS_TFTP_CONTEXT *TftpContext;
+ UINTN PathNameLen;
ASSERT(IS_DEVICE_PATH_NODE (RemainingDevicePath, MESSAGING_DEVICE_PATH, MSG_IPv4_DP));
IPv4DevicePathNode = (IPv4_DEVICE_PATH*)RemainingDevicePath;
@@ -1187,8 +1188,9 @@ BdsTftpLoadImage (
// The Device Path might contain multiple FilePath nodes
PathName = ConvertDevicePathToText ((EFI_DEVICE_PATH_PROTOCOL*)(IPv4DevicePathNode + 1), FALSE, FALSE);
- AsciiFilePath = AllocatePool (StrLen (PathName) + 1);
- UnicodeStrToAsciiStr (PathName, AsciiFilePath);
+ PathNameLen = StrLen (PathName) + 1;
+ AsciiFilePath = AllocatePool (PathNameLen);
+ UnicodeStrToAsciiStrS (PathName, AsciiFilePath, PathNameLen);
//
// Try to get the size of the file in bytes from the server. If it fails,
--
2.7.4
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 6/6] ArmPkg: enable -DDISABLE_NEW_DEPRECATED_INTERFACES
2016-10-24 16:06 [PATCH 0/6] ArmPkg: eliminate calls to deprecated functions Ard Biesheuvel
` (4 preceding siblings ...)
2016-10-24 16:06 ` [PATCH 5/6] ArmPkg/BdsLib: " Ard Biesheuvel
@ 2016-10-24 16:06 ` Ard Biesheuvel
2016-10-25 11:17 ` Laszlo Ersek
2016-10-25 11:32 ` [PATCH 0/6] ArmPkg: eliminate calls to deprecated functions Ryan Harkin
6 siblings, 1 reply; 17+ messages in thread
From: Ard Biesheuvel @ 2016-10-24 16:06 UTC (permalink / raw)
To: edk2-devel, leif.lindholm, lersek; +Cc: 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>
---
ArmPkg/ArmPkg.dsc | 2 ++
1 file changed, 2 insertions(+)
diff --git a/ArmPkg/ArmPkg.dsc b/ArmPkg/ArmPkg.dsc
index 9bb1263a7f96..cd44ec166703 100644
--- a/ArmPkg/ArmPkg.dsc
+++ b/ArmPkg/ArmPkg.dsc
@@ -3,6 +3,7 @@
#
# Copyright (c) 2009 - 2010, Apple Inc. All rights reserved.<BR>
# Copyright (c) 2011 - 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
@@ -36,6 +37,7 @@ [BuildOptions]
RVCT:*_*_ARM_PLATFORM_FLAGS == --cpu Cortex-A15
RELEASE_*_*_CC_FLAGS = -DMDEPKG_NDEBUG
+ *_*_*_CC_FLAGS = -DDISABLE_NEW_DEPRECATED_INTERFACES
[LibraryClasses.common]
BaseLib|MdePkg/Library/BaseLib/BaseLib.inf
--
2.7.4
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 3/6] ArmPkg/LinuxLoader: eliminate calls to deprecated string functions
2016-10-24 16:06 ` [PATCH 3/6] ArmPkg/LinuxLoader: eliminate calls to deprecated string functions Ard Biesheuvel
@ 2016-10-25 10:53 ` Laszlo Ersek
2016-10-25 10:56 ` Ard Biesheuvel
0 siblings, 1 reply; 17+ messages in thread
From: Laszlo Ersek @ 2016-10-25 10:53 UTC (permalink / raw)
To: Ard Biesheuvel, edk2-devel, leif.lindholm
On 10/24/16 18:06, Ard Biesheuvel wrote:
> Remove calls to deprecated string functions like AsciiStrCpy() and
> UnicodeStrToAsciiStr()
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> ArmPkg/Application/LinuxLoader/Arm/LinuxAtag.c | 2 +-
> ArmPkg/Application/LinuxLoader/LinuxLoader.c | 6 ++++--
> 2 files changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/ArmPkg/Application/LinuxLoader/Arm/LinuxAtag.c b/ArmPkg/Application/LinuxLoader/Arm/LinuxAtag.c
> index fd7ee9c8624d..0b3e2489c758 100644
> --- a/ArmPkg/Application/LinuxLoader/Arm/LinuxAtag.c
> +++ b/ArmPkg/Application/LinuxLoader/Arm/LinuxAtag.c
> @@ -72,7 +72,7 @@ SetupCmdlineTag (
> mLinuxKernelCurrentAtag->header.type = ATAG_CMDLINE;
>
> /* place CommandLine into tag */
> - AsciiStrCpy (mLinuxKernelCurrentAtag->body.cmdline_tag.cmdline, CmdLine);
> + AsciiStrCpyS (mLinuxKernelCurrentAtag->body.cmdline_tag.cmdline, LineLength, CmdLine);
>
> // move pointer to next tag
> mLinuxKernelCurrentAtag = next_tag_address (mLinuxKernelCurrentAtag);
Apparently nothing in this file checks if the tags being added still
actually fit in the preallocated space (which is EFI_SIZE_TO_PAGES
(ATAG_MAX_SIZE)). The change does preserve the previous behavior ("copy
the full string").
> diff --git a/ArmPkg/Application/LinuxLoader/LinuxLoader.c b/ArmPkg/Application/LinuxLoader/LinuxLoader.c
> index 70b960b66f0e..76697c3a8c9d 100644
> --- a/ArmPkg/Application/LinuxLoader/LinuxLoader.c
> +++ b/ArmPkg/Application/LinuxLoader/LinuxLoader.c
> @@ -61,6 +61,7 @@ LinuxLoaderEntryPoint (
> LIST_ENTRY *ResourceLink;
> SYSTEM_MEMORY_RESOURCE *Resource;
> EFI_PHYSICAL_ADDRESS SystemMemoryBase;
> + UINTN Length;
>
> Status = gBS->LocateProtocol (
> &gEfiDevicePathFromTextProtocolGuid,
> @@ -182,12 +183,13 @@ LinuxLoaderEntryPoint (
> }
>
> if (LinuxCommandLine != NULL) {
> - AsciiLinuxCommandLine = AllocatePool ((StrLen (LinuxCommandLine) + 1) * sizeof (CHAR8));
> + Length = StrLen (LinuxCommandLine) + 1;
> + AsciiLinuxCommandLine = AllocatePool (Length);
> if (AsciiLinuxCommandLine == NULL) {
> Status = EFI_OUT_OF_RESOURCES;
> goto Error;
> }
> - UnicodeStrToAsciiStr (LinuxCommandLine, AsciiLinuxCommandLine);
> + UnicodeStrToAsciiStrS (LinuxCommandLine, AsciiLinuxCommandLine, Length);
> }
>
> //
>
I prefer to call character counts without the terminating NUL "Length",
and character counts with the terminating NUL "Size", but that's just a
personal preference. (And the rest of this code uses Length differently
already, for example in "LineLength" itself, near the top.)
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Thanks
Laszlo
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/6] ArmPkg/LinuxLoader: eliminate calls to deprecated string functions
2016-10-25 10:53 ` Laszlo Ersek
@ 2016-10-25 10:56 ` Ard Biesheuvel
2016-10-25 11:08 ` Ryan Harkin
0 siblings, 1 reply; 17+ messages in thread
From: Ard Biesheuvel @ 2016-10-25 10:56 UTC (permalink / raw)
To: Laszlo Ersek; +Cc: edk2-devel@lists.01.org, Leif Lindholm
On 25 October 2016 at 11:53, Laszlo Ersek <lersek@redhat.com> wrote:
> On 10/24/16 18:06, Ard Biesheuvel wrote:
>> Remove calls to deprecated string functions like AsciiStrCpy() and
>> UnicodeStrToAsciiStr()
>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>> ArmPkg/Application/LinuxLoader/Arm/LinuxAtag.c | 2 +-
>> ArmPkg/Application/LinuxLoader/LinuxLoader.c | 6 ++++--
>> 2 files changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/ArmPkg/Application/LinuxLoader/Arm/LinuxAtag.c b/ArmPkg/Application/LinuxLoader/Arm/LinuxAtag.c
>> index fd7ee9c8624d..0b3e2489c758 100644
>> --- a/ArmPkg/Application/LinuxLoader/Arm/LinuxAtag.c
>> +++ b/ArmPkg/Application/LinuxLoader/Arm/LinuxAtag.c
>> @@ -72,7 +72,7 @@ SetupCmdlineTag (
>> mLinuxKernelCurrentAtag->header.type = ATAG_CMDLINE;
>>
>> /* place CommandLine into tag */
>> - AsciiStrCpy (mLinuxKernelCurrentAtag->body.cmdline_tag.cmdline, CmdLine);
>> + AsciiStrCpyS (mLinuxKernelCurrentAtag->body.cmdline_tag.cmdline, LineLength, CmdLine);
>>
>> // move pointer to next tag
>> mLinuxKernelCurrentAtag = next_tag_address (mLinuxKernelCurrentAtag);
>
> Apparently nothing in this file checks if the tags being added still
> actually fit in the preallocated space (which is EFI_SIZE_TO_PAGES
> (ATAG_MAX_SIZE)). The change does preserve the previous behavior ("copy
> the full string").
>
The LinuxLoader is an unmaintained piece of junk, and will be removed
as soon as we can. I did notice that none of these ATAG functions
check whether the allocation as a whole is not overrun, but fixing
/that/ goes way beyond what we're willing to do in terms of
maintenance on deprecated code.
>> diff --git a/ArmPkg/Application/LinuxLoader/LinuxLoader.c b/ArmPkg/Application/LinuxLoader/LinuxLoader.c
>> index 70b960b66f0e..76697c3a8c9d 100644
>> --- a/ArmPkg/Application/LinuxLoader/LinuxLoader.c
>> +++ b/ArmPkg/Application/LinuxLoader/LinuxLoader.c
>> @@ -61,6 +61,7 @@ LinuxLoaderEntryPoint (
>> LIST_ENTRY *ResourceLink;
>> SYSTEM_MEMORY_RESOURCE *Resource;
>> EFI_PHYSICAL_ADDRESS SystemMemoryBase;
>> + UINTN Length;
>>
>> Status = gBS->LocateProtocol (
>> &gEfiDevicePathFromTextProtocolGuid,
>> @@ -182,12 +183,13 @@ LinuxLoaderEntryPoint (
>> }
>>
>> if (LinuxCommandLine != NULL) {
>> - AsciiLinuxCommandLine = AllocatePool ((StrLen (LinuxCommandLine) + 1) * sizeof (CHAR8));
>> + Length = StrLen (LinuxCommandLine) + 1;
>> + AsciiLinuxCommandLine = AllocatePool (Length);
>> if (AsciiLinuxCommandLine == NULL) {
>> Status = EFI_OUT_OF_RESOURCES;
>> goto Error;
>> }
>> - UnicodeStrToAsciiStr (LinuxCommandLine, AsciiLinuxCommandLine);
>> + UnicodeStrToAsciiStrS (LinuxCommandLine, AsciiLinuxCommandLine, Length);
>> }
>>
>> //
>>
>
> I prefer to call character counts without the terminating NUL "Length",
> and character counts with the terminating NUL "Size", but that's just a
> personal preference. (And the rest of this code uses Length differently
> already, for example in "LineLength" itself, near the top.)
>
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
>
Thanks,
Ard.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/6] ArmPkg/LinuxLoader: eliminate calls to deprecated string functions
2016-10-25 10:56 ` Ard Biesheuvel
@ 2016-10-25 11:08 ` Ryan Harkin
2016-10-25 11:09 ` Ard Biesheuvel
0 siblings, 1 reply; 17+ messages in thread
From: Ryan Harkin @ 2016-10-25 11:08 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: Laszlo Ersek, edk2-devel@lists.01.org, Leif Lindholm
On 25 October 2016 at 11:56, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> On 25 October 2016 at 11:53, Laszlo Ersek <lersek@redhat.com> wrote:
>> On 10/24/16 18:06, Ard Biesheuvel wrote:
>>> Remove calls to deprecated string functions like AsciiStrCpy() and
>>> UnicodeStrToAsciiStr()
>>>
>>> Contributed-under: TianoCore Contribution Agreement 1.0
>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>> ---
>>> ArmPkg/Application/LinuxLoader/Arm/LinuxAtag.c | 2 +-
>>> ArmPkg/Application/LinuxLoader/LinuxLoader.c | 6 ++++--
>>> 2 files changed, 5 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/ArmPkg/Application/LinuxLoader/Arm/LinuxAtag.c b/ArmPkg/Application/LinuxLoader/Arm/LinuxAtag.c
>>> index fd7ee9c8624d..0b3e2489c758 100644
>>> --- a/ArmPkg/Application/LinuxLoader/Arm/LinuxAtag.c
>>> +++ b/ArmPkg/Application/LinuxLoader/Arm/LinuxAtag.c
>>> @@ -72,7 +72,7 @@ SetupCmdlineTag (
>>> mLinuxKernelCurrentAtag->header.type = ATAG_CMDLINE;
>>>
>>> /* place CommandLine into tag */
>>> - AsciiStrCpy (mLinuxKernelCurrentAtag->body.cmdline_tag.cmdline, CmdLine);
>>> + AsciiStrCpyS (mLinuxKernelCurrentAtag->body.cmdline_tag.cmdline, LineLength, CmdLine);
>>>
>>> // move pointer to next tag
>>> mLinuxKernelCurrentAtag = next_tag_address (mLinuxKernelCurrentAtag);
>>
>> Apparently nothing in this file checks if the tags being added still
>> actually fit in the preallocated space (which is EFI_SIZE_TO_PAGES
>> (ATAG_MAX_SIZE)). The change does preserve the previous behavior ("copy
>> the full string").
>>
>
> The LinuxLoader is an unmaintained piece of junk, and will be removed
> as soon as we can.
Who still uses it? Hikey?
> I did notice that none of these ATAG functions
> check whether the allocation as a whole is not overrun, but fixing
> /that/ goes way beyond what we're willing to do in terms of
> maintenance on deprecated code.
>
>>> diff --git a/ArmPkg/Application/LinuxLoader/LinuxLoader.c b/ArmPkg/Application/LinuxLoader/LinuxLoader.c
>>> index 70b960b66f0e..76697c3a8c9d 100644
>>> --- a/ArmPkg/Application/LinuxLoader/LinuxLoader.c
>>> +++ b/ArmPkg/Application/LinuxLoader/LinuxLoader.c
>>> @@ -61,6 +61,7 @@ LinuxLoaderEntryPoint (
>>> LIST_ENTRY *ResourceLink;
>>> SYSTEM_MEMORY_RESOURCE *Resource;
>>> EFI_PHYSICAL_ADDRESS SystemMemoryBase;
>>> + UINTN Length;
>>>
>>> Status = gBS->LocateProtocol (
>>> &gEfiDevicePathFromTextProtocolGuid,
>>> @@ -182,12 +183,13 @@ LinuxLoaderEntryPoint (
>>> }
>>>
>>> if (LinuxCommandLine != NULL) {
>>> - AsciiLinuxCommandLine = AllocatePool ((StrLen (LinuxCommandLine) + 1) * sizeof (CHAR8));
>>> + Length = StrLen (LinuxCommandLine) + 1;
>>> + AsciiLinuxCommandLine = AllocatePool (Length);
>>> if (AsciiLinuxCommandLine == NULL) {
>>> Status = EFI_OUT_OF_RESOURCES;
>>> goto Error;
>>> }
>>> - UnicodeStrToAsciiStr (LinuxCommandLine, AsciiLinuxCommandLine);
>>> + UnicodeStrToAsciiStrS (LinuxCommandLine, AsciiLinuxCommandLine, Length);
>>> }
>>>
>>> //
>>>
>>
>> I prefer to call character counts without the terminating NUL "Length",
>> and character counts with the terminating NUL "Size", but that's just a
>> personal preference. (And the rest of this code uses Length differently
>> already, for example in "LineLength" itself, near the top.)
>>
>> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
>>
>
> Thanks,
> Ard.
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/6] ArmPkg/LinuxLoader: eliminate calls to deprecated string functions
2016-10-25 11:08 ` Ryan Harkin
@ 2016-10-25 11:09 ` Ard Biesheuvel
0 siblings, 0 replies; 17+ messages in thread
From: Ard Biesheuvel @ 2016-10-25 11:09 UTC (permalink / raw)
To: Ryan Harkin; +Cc: Laszlo Ersek, edk2-devel@lists.01.org, Leif Lindholm
On 25 October 2016 at 12:08, Ryan Harkin <ryan.harkin@linaro.org> wrote:
> On 25 October 2016 at 11:56, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>> On 25 October 2016 at 11:53, Laszlo Ersek <lersek@redhat.com> wrote:
>>> On 10/24/16 18:06, Ard Biesheuvel wrote:
>>>> Remove calls to deprecated string functions like AsciiStrCpy() and
>>>> UnicodeStrToAsciiStr()
>>>>
>>>> Contributed-under: TianoCore Contribution Agreement 1.0
>>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>>> ---
>>>> ArmPkg/Application/LinuxLoader/Arm/LinuxAtag.c | 2 +-
>>>> ArmPkg/Application/LinuxLoader/LinuxLoader.c | 6 ++++--
>>>> 2 files changed, 5 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/ArmPkg/Application/LinuxLoader/Arm/LinuxAtag.c b/ArmPkg/Application/LinuxLoader/Arm/LinuxAtag.c
>>>> index fd7ee9c8624d..0b3e2489c758 100644
>>>> --- a/ArmPkg/Application/LinuxLoader/Arm/LinuxAtag.c
>>>> +++ b/ArmPkg/Application/LinuxLoader/Arm/LinuxAtag.c
>>>> @@ -72,7 +72,7 @@ SetupCmdlineTag (
>>>> mLinuxKernelCurrentAtag->header.type = ATAG_CMDLINE;
>>>>
>>>> /* place CommandLine into tag */
>>>> - AsciiStrCpy (mLinuxKernelCurrentAtag->body.cmdline_tag.cmdline, CmdLine);
>>>> + AsciiStrCpyS (mLinuxKernelCurrentAtag->body.cmdline_tag.cmdline, LineLength, CmdLine);
>>>>
>>>> // move pointer to next tag
>>>> mLinuxKernelCurrentAtag = next_tag_address (mLinuxKernelCurrentAtag);
>>>
>>> Apparently nothing in this file checks if the tags being added still
>>> actually fit in the preallocated space (which is EFI_SIZE_TO_PAGES
>>> (ATAG_MAX_SIZE)). The change does preserve the previous behavior ("copy
>>> the full string").
>>>
>>
>> The LinuxLoader is an unmaintained piece of junk, and will be removed
>> as soon as we can.
>
> Who still uses it? Hikey?
>
I certainly hope not. HiKey is AARCH64, and LinuxLoader is quite badly
broken on that architecture.
>
>> I did notice that none of these ATAG functions
>> check whether the allocation as a whole is not overrun, but fixing
>> /that/ goes way beyond what we're willing to do in terms of
>> maintenance on deprecated code.
>>
>>>> diff --git a/ArmPkg/Application/LinuxLoader/LinuxLoader.c b/ArmPkg/Application/LinuxLoader/LinuxLoader.c
>>>> index 70b960b66f0e..76697c3a8c9d 100644
>>>> --- a/ArmPkg/Application/LinuxLoader/LinuxLoader.c
>>>> +++ b/ArmPkg/Application/LinuxLoader/LinuxLoader.c
>>>> @@ -61,6 +61,7 @@ LinuxLoaderEntryPoint (
>>>> LIST_ENTRY *ResourceLink;
>>>> SYSTEM_MEMORY_RESOURCE *Resource;
>>>> EFI_PHYSICAL_ADDRESS SystemMemoryBase;
>>>> + UINTN Length;
>>>>
>>>> Status = gBS->LocateProtocol (
>>>> &gEfiDevicePathFromTextProtocolGuid,
>>>> @@ -182,12 +183,13 @@ LinuxLoaderEntryPoint (
>>>> }
>>>>
>>>> if (LinuxCommandLine != NULL) {
>>>> - AsciiLinuxCommandLine = AllocatePool ((StrLen (LinuxCommandLine) + 1) * sizeof (CHAR8));
>>>> + Length = StrLen (LinuxCommandLine) + 1;
>>>> + AsciiLinuxCommandLine = AllocatePool (Length);
>>>> if (AsciiLinuxCommandLine == NULL) {
>>>> Status = EFI_OUT_OF_RESOURCES;
>>>> goto Error;
>>>> }
>>>> - UnicodeStrToAsciiStr (LinuxCommandLine, AsciiLinuxCommandLine);
>>>> + UnicodeStrToAsciiStrS (LinuxCommandLine, AsciiLinuxCommandLine, Length);
>>>> }
>>>>
>>>> //
>>>>
>>>
>>> I prefer to call character counts without the terminating NUL "Length",
>>> and character counts with the terminating NUL "Size", but that's just a
>>> personal preference. (And the rest of this code uses Length differently
>>> already, for example in "LineLength" itself, near the top.)
>>>
>>> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
>>>
>>
>> Thanks,
>> Ard.
>> _______________________________________________
>> edk2-devel mailing list
>> edk2-devel@lists.01.org
>> https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 4/6] ArmPkg/SemihostFs: eliminate calls to deprecated string functions
2016-10-24 16:06 ` [PATCH 4/6] ArmPkg/SemihostFs: " Ard Biesheuvel
@ 2016-10-25 11:11 ` Laszlo Ersek
0 siblings, 0 replies; 17+ messages in thread
From: Laszlo Ersek @ 2016-10-25 11:11 UTC (permalink / raw)
To: Ard Biesheuvel, edk2-devel, leif.lindholm
On 10/24/16 18:06, Ard Biesheuvel wrote:
> Remove calls to deprecated string functions like AsciiStrCpy() and
> UnicodeStrToAsciiStr()
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> ArmPkg/Filesystem/SemihostFs/Arm/SemihostFs.c | 20 ++++++++++++--------
> 1 file changed, 12 insertions(+), 8 deletions(-)
>
> diff --git a/ArmPkg/Filesystem/SemihostFs/Arm/SemihostFs.c b/ArmPkg/Filesystem/SemihostFs/Arm/SemihostFs.c
> index 6efdad9ebcce..e79b5cc5cf39 100644
> --- a/ArmPkg/Filesystem/SemihostFs/Arm/SemihostFs.c
> +++ b/ArmPkg/Filesystem/SemihostFs/Arm/SemihostFs.c
> @@ -207,11 +207,12 @@ FileOpen (
> return EFI_WRITE_PROTECTED;
> }
>
> - AsciiFileName = AllocatePool (StrLen (FileName) + 1);
> + Length = StrLen (FileName) + 1;
> + AsciiFileName = AllocatePool (Length);
> if (AsciiFileName == NULL) {
> return EFI_OUT_OF_RESOURCES;
> }
> - UnicodeStrToAsciiStr (FileName, AsciiFileName);
> + UnicodeStrToAsciiStrS (FileName, AsciiFileName, Length);
>
> // Opening '/', '\', '.', or the NULL pathname is trying to open the root directory
> if ((AsciiStrCmp (AsciiFileName, "\\") == 0) ||
Sort of muddles the purpose of the preexistent Length variable, but it's
manageable I think.
> @@ -463,7 +464,7 @@ FileDelete (
> NameSize = AsciiStrLen (Fcb->FileName);
> FileName = AllocatePool (NameSize + 1);
>
> - AsciiStrCpy (FileName, Fcb->FileName);
> + AsciiStrCpyS (FileName, NameSize + 1, Fcb->FileName);
>
> // Close the file if it's open. Disregard return status,
> // since it might give an error if the file isn't open.
Haha, this uses exactly the opposite meanings for Size and Length of
what I do :) Okay.
> @@ -828,8 +829,10 @@ GetFilesystemInfo (
> EFI_FILE_SYSTEM_INFO *Info;
> EFI_STATUS Status;
> UINTN ResultSize;
> + UINTN StringSize;
>
> - ResultSize = SIZE_OF_EFI_FILE_SYSTEM_INFO + StrSize (mSemihostFsLabel);
> + StringSize = StrSize (mSemihostFsLabel);
> + ResultSize = SIZE_OF_EFI_FILE_SYSTEM_INFO + StringSize;
>
> if (*BufferSize >= ResultSize) {
> ZeroMem (Buffer, ResultSize);
> @@ -843,7 +846,7 @@ GetFilesystemInfo (
> Info->FreeSpace = 0;
> Info->BlockSize = 0;
>
> - StrCpy (Info->VolumeLabel, mSemihostFsLabel);
> + StrCpyS (Info->VolumeLabel, StringSize, mSemihostFsLabel);
> } else {
> Status = EFI_BUFFER_TOO_SMALL;
> }
This is wrong:
- StrSize() "Returns the size of a Null-terminated Unicode string in
bytes, including the Null terminator",
- but for StrCpyS(), "DestMax" is "The maximum number of Destination
Unicode char, including terminating null char."
I suggest to use CopyMem() (rather than divide).
> @@ -903,7 +906,7 @@ FileGetInfo (
> ResultSize = StrSize (mSemihostFsLabel);
>
> if (*BufferSize >= ResultSize) {
> - StrCpy (Buffer, mSemihostFsLabel);
> + StrCpyS (Buffer, *BufferSize, mSemihostFsLabel);
> Status = EFI_SUCCESS;
> } else {
> Status = EFI_BUFFER_TOO_SMALL;
Also wrong; please use CopyMem().
> @@ -963,11 +966,12 @@ SetFileInfo (
> return EFI_ACCESS_DENIED;
> }
>
> - AsciiFileName = AllocatePool (StrLen (Info->FileName) + 1);
> + Length = StrLen (Info->FileName) + 1;
> + AsciiFileName = AllocatePool (Length);
> if (AsciiFileName == NULL) {
> return EFI_OUT_OF_RESOURCES;
> }
> - UnicodeStrToAsciiStr (Info->FileName, AsciiFileName);
> + UnicodeStrToAsciiStrS (Info->FileName, AsciiFileName, Length);
>
> FileSizeIsDifferent = (Info->FileSize != Fcb->Info.FileSize);
> FileNameIsDifferent = (AsciiStrCmp (AsciiFileName, Fcb->FileName) != 0);
>
This hunk looks okay.
Thanks
Laszlo
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 5/6] ArmPkg/BdsLib: eliminate calls to deprecated string functions
2016-10-24 16:06 ` [PATCH 5/6] ArmPkg/BdsLib: " Ard Biesheuvel
@ 2016-10-25 11:16 ` Laszlo Ersek
0 siblings, 0 replies; 17+ messages in thread
From: Laszlo Ersek @ 2016-10-25 11:16 UTC (permalink / raw)
To: Ard Biesheuvel, edk2-devel, leif.lindholm
On 10/24/16 18:06, Ard Biesheuvel wrote:
> Remove calls to deprecated string functions like AsciiStrCpy() and
> UnicodeStrToAsciiStr()
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> ArmPkg/Library/BdsLib/BdsFilePath.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/ArmPkg/Library/BdsLib/BdsFilePath.c b/ArmPkg/Library/BdsLib/BdsFilePath.c
> index aefeaed4ffb5..351a8bd8edf4 100644
> --- a/ArmPkg/Library/BdsLib/BdsFilePath.c
> +++ b/ArmPkg/Library/BdsLib/BdsFilePath.c
> @@ -964,7 +964,7 @@ Mtftp4CheckPacket (
> Step = (Context->DownloadedNbOfBytes * TFTP_PROGRESS_SLIDER_STEPS) / Context->FileSize;
> if (Step > LastStep) {
> Print (mTftpProgressDelete);
> - StrCpy (Progress, mTftpProgressFrame);
> + StrCpyS (Progress, sizeof (Progress), mTftpProgressFrame);
> for (Index = 1; Index < Step; Index++) {
> Progress[Index] = L'=';
> }
Same problem; please use CopyMem(), or introduce an ARRAY_SIZE() macro,
and use that (which divides by the size of the element at offset zero).
> @@ -1044,6 +1044,7 @@ BdsTftpLoadImage (
> UINT64 FileSize;
> UINT64 TftpBufferSize;
> BDS_TFTP_CONTEXT *TftpContext;
> + UINTN PathNameLen;
>
> ASSERT(IS_DEVICE_PATH_NODE (RemainingDevicePath, MESSAGING_DEVICE_PATH, MSG_IPv4_DP));
> IPv4DevicePathNode = (IPv4_DEVICE_PATH*)RemainingDevicePath;
> @@ -1187,8 +1188,9 @@ BdsTftpLoadImage (
>
> // The Device Path might contain multiple FilePath nodes
> PathName = ConvertDevicePathToText ((EFI_DEVICE_PATH_PROTOCOL*)(IPv4DevicePathNode + 1), FALSE, FALSE);
> - AsciiFilePath = AllocatePool (StrLen (PathName) + 1);
> - UnicodeStrToAsciiStr (PathName, AsciiFilePath);
> + PathNameLen = StrLen (PathName) + 1;
> + AsciiFilePath = AllocatePool (PathNameLen);
> + UnicodeStrToAsciiStrS (PathName, AsciiFilePath, PathNameLen);
>
> //
> // Try to get the size of the file in bytes from the server. If it fails,
>
This hunk looks fine.
Thanks
Laszlo
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 6/6] ArmPkg: enable -DDISABLE_NEW_DEPRECATED_INTERFACES
2016-10-24 16:06 ` [PATCH 6/6] ArmPkg: enable -DDISABLE_NEW_DEPRECATED_INTERFACES Ard Biesheuvel
@ 2016-10-25 11:17 ` Laszlo Ersek
0 siblings, 0 replies; 17+ messages in thread
From: Laszlo Ersek @ 2016-10-25 11:17 UTC (permalink / raw)
To: Ard Biesheuvel, edk2-devel, leif.lindholm
On 10/24/16 18:06, 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>
> ---
> ArmPkg/ArmPkg.dsc | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/ArmPkg/ArmPkg.dsc b/ArmPkg/ArmPkg.dsc
> index 9bb1263a7f96..cd44ec166703 100644
> --- a/ArmPkg/ArmPkg.dsc
> +++ b/ArmPkg/ArmPkg.dsc
> @@ -3,6 +3,7 @@
> #
> # Copyright (c) 2009 - 2010, Apple Inc. All rights reserved.<BR>
> # Copyright (c) 2011 - 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
> @@ -36,6 +37,7 @@ [BuildOptions]
> RVCT:*_*_ARM_PLATFORM_FLAGS == --cpu Cortex-A15
>
> RELEASE_*_*_CC_FLAGS = -DMDEPKG_NDEBUG
> + *_*_*_CC_FLAGS = -DDISABLE_NEW_DEPRECATED_INTERFACES
>
> [LibraryClasses.common]
> BaseLib|MdePkg/Library/BaseLib/BaseLib.inf
>
Acked-by: Laszlo Ersek <lersek@redhat.com>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 0/6] ArmPkg: eliminate calls to deprecated functions
2016-10-24 16:06 [PATCH 0/6] ArmPkg: eliminate calls to deprecated functions Ard Biesheuvel
` (5 preceding siblings ...)
2016-10-24 16:06 ` [PATCH 6/6] ArmPkg: enable -DDISABLE_NEW_DEPRECATED_INTERFACES Ard Biesheuvel
@ 2016-10-25 11:32 ` Ryan Harkin
6 siblings, 0 replies; 17+ messages in thread
From: Ryan Harkin @ 2016-10-25 11:32 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: edk2-devel-01, Leif Lindholm, Laszlo Ersek
On 24 October 2016 at 17:06, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> This series prepares ArmPkg for building with the preprocess symbol
> DISABLE_NEW_DEPRECATED_INTERFACES defined, by adding missing components
> to ArmPkg (#1), fixing broken code or code that relies on deprecated
> functionality (#2 - #5), and finally adds -DDISABLE_NEW_DEPRECATED_INTERFACES
> to the CC flags for all build types, toolchains and architectures.
>
> Bug: https://bugzilla.tianocore.org/show_bug.cgi?id=164
>
> Ard Biesheuvel (6):
> ArmPkg: add missing components
> ArmPkg/ArmCortexA9Lib RVCT: remove incompatible GCC include
> ArmPkg/LinuxLoader: eliminate calls to deprecated string functions
> ArmPkg/SemihostFs: eliminate calls to deprecated string functions
> ArmPkg/BdsLib: eliminate calls to deprecated string functions
> ArmPkg: enable -DDISABLE_NEW_DEPRECATED_INTERFACES
>
I tested the series on FVP AEMv8 & Foundation, TC2 and Juno R0/1/2:
Tested-by: Ryan Harkin <ryan.harkin@linaro.org>
> ArmPkg/Application/LinuxLoader/Arm/LinuxAtag.c | 2 +-
> ArmPkg/Application/LinuxLoader/LinuxLoader.c | 6 ++++--
> ArmPkg/ArmPkg.dsc | 13 +++++++++++++
> ArmPkg/Drivers/ArmCpuLib/ArmCortexA9Lib/ArmCortexA9Helper.asm | 4 ----
> ArmPkg/Filesystem/SemihostFs/Arm/SemihostFs.c | 20 ++++++++++++--------
> ArmPkg/Library/BdsLib/BdsFilePath.c | 8 +++++---
> 6 files changed, 35 insertions(+), 18 deletions(-)
>
> --
> 2.7.4
>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/6] ArmPkg: add missing components
2016-10-24 16:06 ` [PATCH 1/6] ArmPkg: add missing components Ard Biesheuvel
@ 2016-10-25 12:51 ` Leif Lindholm
0 siblings, 0 replies; 17+ messages in thread
From: Leif Lindholm @ 2016-10-25 12:51 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: edk2-devel, lersek
On Mon, Oct 24, 2016 at 05:06:41PM +0100, Ard Biesheuvel wrote:
> ArmPkg.dsc was a bit out of date, and some modules added over the past
> years had not been added to its [Components] section yet.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
> ---
> ArmPkg/ArmPkg.dsc | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/ArmPkg/ArmPkg.dsc b/ArmPkg/ArmPkg.dsc
> index 6a8ff7e621d7..9bb1263a7f96 100644
> --- a/ArmPkg/ArmPkg.dsc
> +++ b/ArmPkg/ArmPkg.dsc
> @@ -143,6 +143,16 @@ [Components.common]
>
> ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
>
> + ArmPkg/Drivers/ArmPciCpuIo2Dxe/ArmPciCpuIo2Dxe.inf
> + ArmPkg/Library/ArmArchTimerLib/ArmArchTimerLib.inf
> + ArmPkg/Library/ArmGicArchLib/ArmGicArchLib.inf
> + ArmPkg/Library/ArmGicArchSecLib/ArmGicArchSecLib.inf
> + ArmPkg/Library/ArmLib/ArmBaseLib.inf
> + ArmPkg/Library/ArmSoftFloatLib/ArmSoftFloatLib.inf
> + ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerLibBase.inf
> + ArmPkg/Library/PeiServicesTablePointerLib/PeiServicesTablePointerLib.inf
> + ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> +
> [Components.ARM]
> ArmPkg/Drivers/ArmCpuLib/ArmCortexA8Lib/ArmCortexA8Lib.inf
> ArmPkg/Drivers/ArmCpuLib/ArmCortexA9Lib/ArmCortexA9Lib.inf
> @@ -151,3 +161,4 @@ [Components.ARM]
> [Components.AARCH64]
> ArmPkg/Drivers/ArmCpuLib/ArmCortexAEMv8Lib/ArmCortexAEMv8Lib.inf
> ArmPkg/Drivers/ArmCpuLib/ArmCortexA5xLib/ArmCortexA5xLib.inf
> + ArmPkg/Library/ArmMmuLib/ArmMmuPeiLib.inf
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/6] ArmPkg/ArmCortexA9Lib RVCT: remove incompatible GCC include
2016-10-24 16:06 ` [PATCH 2/6] ArmPkg/ArmCortexA9Lib RVCT: remove incompatible GCC include Ard Biesheuvel
@ 2016-10-25 12:53 ` Leif Lindholm
0 siblings, 0 replies; 17+ messages in thread
From: Leif Lindholm @ 2016-10-25 12:53 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: edk2-devel, lersek
On Mon, Oct 24, 2016 at 05:06:42PM +0100, Ard Biesheuvel wrote:
> Drop the include of AsmMacroIoLib.h, which contains GCC preprocessor macros
> that RVCT does not use or require, given it has its own AsmMacroIoLib.inc
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Not verified, but looks sensible, so:
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
...ish
> ---
> ArmPkg/Drivers/ArmCpuLib/ArmCortexA9Lib/ArmCortexA9Helper.asm | 4 ----
> 1 file changed, 4 deletions(-)
>
> diff --git a/ArmPkg/Drivers/ArmCpuLib/ArmCortexA9Lib/ArmCortexA9Helper.asm b/ArmPkg/Drivers/ArmCpuLib/ArmCortexA9Lib/ArmCortexA9Helper.asm
> index 1417c9a78204..882f25b780cd 100644
> --- a/ArmPkg/Drivers/ArmCpuLib/ArmCortexA9Lib/ArmCortexA9Helper.asm
> +++ b/ArmPkg/Drivers/ArmCpuLib/ArmCortexA9Lib/ArmCortexA9Helper.asm
> @@ -11,10 +11,6 @@
> //
> //
>
> -#include <AsmMacroIoLib.h>
> -#include <Library/ArmCpuLib.h>
> -#include <Chipset/ArmCortexA9.h>
> -
> INCLUDE AsmMacroExport.inc
> INCLUDE AsmMacroIoLib.inc
>
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2016-10-25 12:53 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-10-24 16:06 [PATCH 0/6] ArmPkg: eliminate calls to deprecated functions Ard Biesheuvel
2016-10-24 16:06 ` [PATCH 1/6] ArmPkg: add missing components Ard Biesheuvel
2016-10-25 12:51 ` Leif Lindholm
2016-10-24 16:06 ` [PATCH 2/6] ArmPkg/ArmCortexA9Lib RVCT: remove incompatible GCC include Ard Biesheuvel
2016-10-25 12:53 ` Leif Lindholm
2016-10-24 16:06 ` [PATCH 3/6] ArmPkg/LinuxLoader: eliminate calls to deprecated string functions Ard Biesheuvel
2016-10-25 10:53 ` Laszlo Ersek
2016-10-25 10:56 ` Ard Biesheuvel
2016-10-25 11:08 ` Ryan Harkin
2016-10-25 11:09 ` Ard Biesheuvel
2016-10-24 16:06 ` [PATCH 4/6] ArmPkg/SemihostFs: " Ard Biesheuvel
2016-10-25 11:11 ` Laszlo Ersek
2016-10-24 16:06 ` [PATCH 5/6] ArmPkg/BdsLib: " Ard Biesheuvel
2016-10-25 11:16 ` Laszlo Ersek
2016-10-24 16:06 ` [PATCH 6/6] ArmPkg: enable -DDISABLE_NEW_DEPRECATED_INTERFACES Ard Biesheuvel
2016-10-25 11:17 ` Laszlo Ersek
2016-10-25 11:32 ` [PATCH 0/6] ArmPkg: eliminate calls to deprecated functions Ryan Harkin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox