* [PATCH v2 0/6] BaseTools: get rid of MAX_UINTN
@ 2018-11-30 22:45 Ard Biesheuvel
2018-11-30 22:45 ` [PATCH v2 1/6] BaseTools/CommonLib: avoid using 'native' word size in IP address handling Ard Biesheuvel
` (6 more replies)
0 siblings, 7 replies; 28+ messages in thread
From: Ard Biesheuvel @ 2018-11-30 22:45 UTC (permalink / raw)
To: edk2-devel
Cc: Ard Biesheuvel, Laszlo Ersek, Yonghong Zhu, Liming Gao, Bob Feng,
Jaben Carsey
There should be no reason for the build tools to care about the native
word size of a particular target, so relying on a definition of MAX_UINTN
is definitely wrong, and most likely inaccurate on 32-bit build hosts.
So refactor the code in CommonLib and DevicePath so we no longer rely
on this definition.
Changes since v1:
- miss type change in #1 causing a build failure on MSVC
- add acks from Jaben
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Yonghong Zhu <yonghong.zhu@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Bob Feng <bob.c.feng@intel.com>
Cc: Jaben Carsey <jaben.carsey@intel.com>
Ard Biesheuvel (6):
BaseTools/CommonLib: avoid using 'native' word size in IP address
handling
BaseTools/CommonLib: use explicit 64-bit type in Strtoi()
BaseTools/DevicePath: use explicit 64-bit number parsing routines
BaseTools/DevicePath: use MAX_UINT16 as default device path max size
BaseTools/CommonLib: get rid of 'native' type string parsing routines
BaseTools/CommonLib: drop definition of MAX_UINTN
BaseTools/Source/C/Common/CommonLib.h | 25 ---
BaseTools/Source/C/Common/CommonLib.c | 206 ++----------------
.../Source/C/DevicePath/DevicePathFromText.c | 4 +-
.../Source/C/DevicePath/DevicePathUtilities.c | 4 +-
4 files changed, 25 insertions(+), 214 deletions(-)
--
2.19.1
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v2 1/6] BaseTools/CommonLib: avoid using 'native' word size in IP address handling
2018-11-30 22:45 [PATCH v2 0/6] BaseTools: get rid of MAX_UINTN Ard Biesheuvel
@ 2018-11-30 22:45 ` Ard Biesheuvel
2018-12-03 10:25 ` Philippe Mathieu-Daudé
2018-12-03 12:50 ` Laszlo Ersek
2018-11-30 22:45 ` [PATCH v2 2/6] BaseTools/CommonLib: use explicit 64-bit type in Strtoi() Ard Biesheuvel
` (5 subsequent siblings)
6 siblings, 2 replies; 28+ messages in thread
From: Ard Biesheuvel @ 2018-11-30 22:45 UTC (permalink / raw)
To: edk2-devel
Cc: Ard Biesheuvel, Laszlo Ersek, Yonghong Zhu, Liming Gao, Bob Feng,
Jaben Carsey
In the context of the BaseTools, there is no such thing as a native word
size, given that the same set of tools may be used to build a firmware
image consisting of both 32-bit and 64-bit modules.
So update StrToIpv4Address() and StrToIpv6Address() to use UINT64
types instead of UINTN types when parsing strings.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
BaseTools/Source/C/Common/CommonLib.c | 28 ++++++++++----------
1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/BaseTools/Source/C/Common/CommonLib.c b/BaseTools/Source/C/Common/CommonLib.c
index 618aadac781a..7c559bc3c222 100644
--- a/BaseTools/Source/C/Common/CommonLib.c
+++ b/BaseTools/Source/C/Common/CommonLib.c
@@ -1785,7 +1785,7 @@ StrToIpv4Address (
{
RETURN_STATUS Status;
UINTN AddressIndex;
- UINTN Uintn;
+ UINT64 Uint64;
EFI_IPv4_ADDRESS LocalAddress;
UINT8 LocalPrefixLength;
CHAR16 *Pointer;
@@ -1812,7 +1812,7 @@ StrToIpv4Address (
//
// Get D or P.
//
- Status = StrDecimalToUintnS ((CONST CHAR16 *) Pointer, &Pointer, &Uintn);
+ Status = StrDecimalToUint64S ((CONST CHAR16 *) Pointer, &Pointer, &Uint64);
if (RETURN_ERROR (Status)) {
return RETURN_UNSUPPORTED;
}
@@ -1820,18 +1820,18 @@ StrToIpv4Address (
//
// It's P.
//
- if (Uintn > 32) {
+ if (Uint64 > 32) {
return RETURN_UNSUPPORTED;
}
- LocalPrefixLength = (UINT8) Uintn;
+ LocalPrefixLength = (UINT8) Uint64;
} else {
//
// It's D.
//
- if (Uintn > MAX_UINT8) {
+ if (Uint64 > MAX_UINT8) {
return RETURN_UNSUPPORTED;
}
- LocalAddress.Addr[AddressIndex] = (UINT8) Uintn;
+ LocalAddress.Addr[AddressIndex] = (UINT8) Uint64;
AddressIndex++;
}
@@ -1888,7 +1888,7 @@ StrToIpv6Address (
{
RETURN_STATUS Status;
UINTN AddressIndex;
- UINTN Uintn;
+ UINT64 Uint64;
EFI_IPv6_ADDRESS LocalAddress;
UINT8 LocalPrefixLength;
CONST CHAR16 *Pointer;
@@ -1969,7 +1969,7 @@ StrToIpv6Address (
//
// Get X.
//
- Status = StrHexToUintnS (Pointer, &End, &Uintn);
+ Status = StrHexToUint64S (Pointer, &End, &Uint64);
if (RETURN_ERROR (Status) || End - Pointer > 4) {
//
// Number of hexadecimal digit characters is no more than 4.
@@ -1978,24 +1978,24 @@ StrToIpv6Address (
}
Pointer = End;
//
- // Uintn won't exceed MAX_UINT16 if number of hexadecimal digit characters is no more than 4.
+ // Uint64 won't exceed MAX_UINT16 if number of hexadecimal digit characters is no more than 4.
//
ASSERT (AddressIndex + 1 < ARRAY_SIZE (Address->Addr));
- LocalAddress.Addr[AddressIndex] = (UINT8) ((UINT16) Uintn >> 8);
- LocalAddress.Addr[AddressIndex + 1] = (UINT8) Uintn;
+ LocalAddress.Addr[AddressIndex] = (UINT8) ((UINT16) Uint64 >> 8);
+ LocalAddress.Addr[AddressIndex + 1] = (UINT8) Uint64;
AddressIndex += 2;
} else {
//
// Get P, then exit the loop.
//
- Status = StrDecimalToUintnS (Pointer, &End, &Uintn);
- if (RETURN_ERROR (Status) || End == Pointer || Uintn > 128) {
+ Status = StrDecimalToUint64S (Pointer, &End, &Uint64);
+ if (RETURN_ERROR (Status) || End == Pointer || Uint64 > 128) {
//
// Prefix length should not exceed 128.
//
return RETURN_UNSUPPORTED;
}
- LocalPrefixLength = (UINT8) Uintn;
+ LocalPrefixLength = (UINT8) Uint64;
Pointer = End;
break;
}
--
2.19.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v2 2/6] BaseTools/CommonLib: use explicit 64-bit type in Strtoi()
2018-11-30 22:45 [PATCH v2 0/6] BaseTools: get rid of MAX_UINTN Ard Biesheuvel
2018-11-30 22:45 ` [PATCH v2 1/6] BaseTools/CommonLib: avoid using 'native' word size in IP address handling Ard Biesheuvel
@ 2018-11-30 22:45 ` Ard Biesheuvel
2018-12-03 10:25 ` Philippe Mathieu-Daudé
2018-12-03 12:52 ` Laszlo Ersek
2018-11-30 22:45 ` [PATCH v2 3/6] BaseTools/DevicePath: use explicit 64-bit number parsing routines Ard Biesheuvel
` (4 subsequent siblings)
6 siblings, 2 replies; 28+ messages in thread
From: Ard Biesheuvel @ 2018-11-30 22:45 UTC (permalink / raw)
To: edk2-devel
Cc: Ard Biesheuvel, Laszlo Ersek, Yonghong Zhu, Liming Gao, Bob Feng,
Jaben Carsey
Don't use the native word size string to number parsing routines,
but instead, use the 64-bit one and cast to UINTN.
Currently, the only user is in Source/C/DevicePath/DevicePathFromText.c
which takes care to use Strtoi64 () unless it assumes the value fits
in 32-bit, so this change is a no-op even on 32-bit build hosts.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Jaben Carsey <jaben.carsey@intel.com>
---
BaseTools/Source/C/Common/CommonLib.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/BaseTools/Source/C/Common/CommonLib.c b/BaseTools/Source/C/Common/CommonLib.c
index 7c559bc3c222..4a28f635f3a8 100644
--- a/BaseTools/Source/C/Common/CommonLib.c
+++ b/BaseTools/Source/C/Common/CommonLib.c
@@ -2252,9 +2252,9 @@ Strtoi (
)
{
if (IsHexStr (Str)) {
- return StrHexToUintn (Str);
+ return (UINTN)StrHexToUint64 (Str);
} else {
- return StrDecimalToUintn (Str);
+ return (UINTN)StrDecimalToUint64 (Str);
}
}
--
2.19.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v2 3/6] BaseTools/DevicePath: use explicit 64-bit number parsing routines
2018-11-30 22:45 [PATCH v2 0/6] BaseTools: get rid of MAX_UINTN Ard Biesheuvel
2018-11-30 22:45 ` [PATCH v2 1/6] BaseTools/CommonLib: avoid using 'native' word size in IP address handling Ard Biesheuvel
2018-11-30 22:45 ` [PATCH v2 2/6] BaseTools/CommonLib: use explicit 64-bit type in Strtoi() Ard Biesheuvel
@ 2018-11-30 22:45 ` Ard Biesheuvel
2018-12-03 10:25 ` Philippe Mathieu-Daudé
2018-12-03 12:55 ` Laszlo Ersek
2018-11-30 22:45 ` [PATCH v2 4/6] BaseTools/DevicePath: use MAX_UINT16 as default device path max size Ard Biesheuvel
` (3 subsequent siblings)
6 siblings, 2 replies; 28+ messages in thread
From: Ard Biesheuvel @ 2018-11-30 22:45 UTC (permalink / raw)
To: edk2-devel
Cc: Ard Biesheuvel, Laszlo Ersek, Yonghong Zhu, Liming Gao, Bob Feng,
Jaben Carsey
Replace invocations of StrHexToUintn() with StrHexToUint64(), so
that we can drop the former.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Jaben Carsey <jaben.carsey@intel.com>
---
BaseTools/Source/C/DevicePath/DevicePathFromText.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/BaseTools/Source/C/DevicePath/DevicePathFromText.c b/BaseTools/Source/C/DevicePath/DevicePathFromText.c
index 555efa1acdde..6151926af9aa 100644
--- a/BaseTools/Source/C/DevicePath/DevicePathFromText.c
+++ b/BaseTools/Source/C/DevicePath/DevicePathFromText.c
@@ -520,7 +520,7 @@ EisaIdFromText (
return (((Text[0] - 'A' + 1) & 0x1f) << 10)
+ (((Text[1] - 'A' + 1) & 0x1f) << 5)
+ (((Text[2] - 'A' + 1) & 0x1f) << 0)
- + (UINT32) (StrHexToUintn (&Text[3]) << 16)
+ + (UINT32) (StrHexToUint64 (&Text[3]) << 16)
;
}
@@ -1506,7 +1506,7 @@ DevPathFromTextNVMe (
Index = sizeof (Nvme->NamespaceUuid) / sizeof (UINT8);
while (Index-- != 0) {
- Uuid[Index] = (UINT8) StrHexToUintn (SplitStr (&NamespaceUuidStr, L'-'));
+ Uuid[Index] = (UINT8) StrHexToUint64 (SplitStr (&NamespaceUuidStr, L'-'));
}
return (EFI_DEVICE_PATH_PROTOCOL *) Nvme;
--
2.19.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v2 4/6] BaseTools/DevicePath: use MAX_UINT16 as default device path max size
2018-11-30 22:45 [PATCH v2 0/6] BaseTools: get rid of MAX_UINTN Ard Biesheuvel
` (2 preceding siblings ...)
2018-11-30 22:45 ` [PATCH v2 3/6] BaseTools/DevicePath: use explicit 64-bit number parsing routines Ard Biesheuvel
@ 2018-11-30 22:45 ` Ard Biesheuvel
2018-12-03 13:05 ` Laszlo Ersek
2018-11-30 22:45 ` [PATCH v2 5/6] BaseTools/CommonLib: get rid of 'native' type string parsing routines Ard Biesheuvel
` (2 subsequent siblings)
6 siblings, 1 reply; 28+ messages in thread
From: Ard Biesheuvel @ 2018-11-30 22:45 UTC (permalink / raw)
To: edk2-devel
Cc: Ard Biesheuvel, Laszlo Ersek, Yonghong Zhu, Liming Gao, Bob Feng,
Jaben Carsey
Replace the default size limit of IsDevicePathValid() with a value
that does not depend on the native word size of the build host.
64 KB seems sufficient as the upper bound of a device path handled
by UEFI.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Jaben Carsey <jaben.carsey@intel.com>
---
BaseTools/Source/C/DevicePath/DevicePathUtilities.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/BaseTools/Source/C/DevicePath/DevicePathUtilities.c b/BaseTools/Source/C/DevicePath/DevicePathUtilities.c
index d4ec2742b7c8..ba7f83e53070 100644
--- a/BaseTools/Source/C/DevicePath/DevicePathUtilities.c
+++ b/BaseTools/Source/C/DevicePath/DevicePathUtilities.c
@@ -62,7 +62,7 @@ IsDevicePathValid (
ASSERT (DevicePath != NULL);
if (MaxSize == 0) {
- MaxSize = MAX_UINTN;
+ MaxSize = MAX_UINT16;
}
//
@@ -78,7 +78,7 @@ IsDevicePathValid (
return FALSE;
}
- if (NodeLength > MAX_UINTN - Size) {
+ if (NodeLength > MAX_UINT16 - Size) {
return FALSE;
}
Size += NodeLength;
--
2.19.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v2 5/6] BaseTools/CommonLib: get rid of 'native' type string parsing routines
2018-11-30 22:45 [PATCH v2 0/6] BaseTools: get rid of MAX_UINTN Ard Biesheuvel
` (3 preceding siblings ...)
2018-11-30 22:45 ` [PATCH v2 4/6] BaseTools/DevicePath: use MAX_UINT16 as default device path max size Ard Biesheuvel
@ 2018-11-30 22:45 ` Ard Biesheuvel
2018-12-03 10:27 ` Philippe Mathieu-Daudé
2018-12-03 13:08 ` Laszlo Ersek
2018-11-30 22:45 ` [PATCH v2 6/6] BaseTools/CommonLib: drop definition of MAX_UINTN Ard Biesheuvel
2018-12-05 0:04 ` [PATCH v2 0/6] BaseTools: get rid " Gao, Liming
6 siblings, 2 replies; 28+ messages in thread
From: Ard Biesheuvel @ 2018-11-30 22:45 UTC (permalink / raw)
To: edk2-devel
Cc: Ard Biesheuvel, Laszlo Ersek, Yonghong Zhu, Liming Gao, Bob Feng,
Jaben Carsey
Parsing a string into an integer variable of the native word size
is not defined for the BaseTools, since the same tools may be used
to build firmware for different targets with different native word
sizes.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
BaseTools/Source/C/Common/CommonLib.h | 24 ---
BaseTools/Source/C/Common/CommonLib.c | 174 +-------------------
2 files changed, 5 insertions(+), 193 deletions(-)
diff --git a/BaseTools/Source/C/Common/CommonLib.h b/BaseTools/Source/C/Common/CommonLib.h
index fa10fac4682a..6930d9227b87 100644
--- a/BaseTools/Source/C/Common/CommonLib.h
+++ b/BaseTools/Source/C/Common/CommonLib.h
@@ -250,16 +250,6 @@ StrSize (
CONST CHAR16 *String
);
-UINTN
-StrHexToUintn (
- CONST CHAR16 *String
- );
-
-UINTN
-StrDecimalToUintn (
- CONST CHAR16 *String
- );
-
UINT64
StrHexToUint64 (
CONST CHAR16 *String
@@ -277,13 +267,6 @@ StrHexToUint64S (
UINT64 *Data
);
-RETURN_STATUS
-StrHexToUintnS (
- CONST CHAR16 *String,
- CHAR16 **EndPointer, OPTIONAL
- UINTN *Data
- );
-
RETURN_STATUS
StrDecimalToUint64S (
CONST CHAR16 *String,
@@ -291,13 +274,6 @@ StrDecimalToUint64S (
UINT64 *Data
);
-RETURN_STATUS
-StrDecimalToUintnS (
- CONST CHAR16 *String,
- CHAR16 **EndPointer, OPTIONAL
- UINTN *Data
- );
-
VOID *
ReallocatePool (
UINTN OldSize,
diff --git a/BaseTools/Source/C/Common/CommonLib.c b/BaseTools/Source/C/Common/CommonLib.c
index 4a28f635f3a8..42dfa821624d 100644
--- a/BaseTools/Source/C/Common/CommonLib.c
+++ b/BaseTools/Source/C/Common/CommonLib.c
@@ -882,72 +882,6 @@ InternalSafeStringNoStrOverlap (
return !InternalSafeStringIsOverlap (Str1, Size1 * sizeof(CHAR16), Str2, Size2 * sizeof(CHAR16));
}
-RETURN_STATUS
-StrDecimalToUintnS (
- CONST CHAR16 *String,
- CHAR16 **EndPointer, OPTIONAL
- UINTN *Data
- )
-{
- ASSERT (((UINTN) String & BIT0) == 0);
-
- //
- // 1. Neither String nor Data shall be a null pointer.
- //
- SAFE_STRING_CONSTRAINT_CHECK ((String != NULL), RETURN_INVALID_PARAMETER);
- SAFE_STRING_CONSTRAINT_CHECK ((Data != NULL), RETURN_INVALID_PARAMETER);
-
- //
- // 2. The length of String shall not be greater than RSIZE_MAX.
- //
- if (RSIZE_MAX != 0) {
- SAFE_STRING_CONSTRAINT_CHECK ((StrnLenS (String, RSIZE_MAX + 1) <= RSIZE_MAX), RETURN_INVALID_PARAMETER);
- }
-
- if (EndPointer != NULL) {
- *EndPointer = (CHAR16 *) String;
- }
-
- //
- // Ignore the pad spaces (space or tab)
- //
- while ((*String == L' ') || (*String == L'\t')) {
- String++;
- }
-
- //
- // Ignore leading Zeros after the spaces
- //
- while (*String == L'0') {
- String++;
- }
-
- *Data = 0;
-
- while (InternalIsDecimalDigitCharacter (*String)) {
- //
- // If the number represented by String overflows according to the range
- // defined by UINTN, then MAX_UINTN is stored in *Data and
- // RETURN_UNSUPPORTED is returned.
- //
- if (*Data > ((MAX_UINTN - (*String - L'0')) / 10)) {
- *Data = MAX_UINTN;
- if (EndPointer != NULL) {
- *EndPointer = (CHAR16 *) String;
- }
- return RETURN_UNSUPPORTED;
- }
-
- *Data = *Data * 10 + (*String - L'0');
- String++;
- }
-
- if (EndPointer != NULL) {
- *EndPointer = (CHAR16 *) String;
- }
- return RETURN_SUCCESS;
-}
-
/**
Convert a Null-terminated Unicode decimal string to a value of type UINT64.
@@ -1064,9 +998,9 @@ StrDecimalToUint64S (
/**
Convert a Null-terminated Unicode hexadecimal string to a value of type
- UINTN.
+ UINT64.
- This function outputs a value of type UINTN by interpreting the contents of
+ This function outputs a value of type UINT64 by interpreting the contents of
the Unicode string specified by String as a hexadecimal number. The format of
the input Unicode string String is:
@@ -1091,8 +1025,8 @@ StrDecimalToUint64S (
If String has no valid hexadecimal digits in the above format, then 0 is
stored at the location pointed to by Data.
- If the number represented by String exceeds the range defined by UINTN, then
- MAX_UINTN is stored at the location pointed to by Data.
+ If the number represented by String exceeds the range defined by UINT64, then
+ MAX_UINT64 is stored at the location pointed to by Data.
If EndPointer is not NULL, a pointer to the character that stopped the scan
is stored at the location pointed to by EndPointer. If String has no valid
@@ -1112,86 +1046,10 @@ StrDecimalToUint64S (
characters, not including the
Null-terminator.
@retval RETURN_UNSUPPORTED If the number represented by String exceeds
- the range defined by UINTN.
+ the range defined by UINT64.
**/
RETURN_STATUS
-StrHexToUintnS (
- CONST CHAR16 *String,
- CHAR16 **EndPointer, OPTIONAL
- UINTN *Data
- )
-{
- ASSERT (((UINTN) String & BIT0) == 0);
-
- //
- // 1. Neither String nor Data shall be a null pointer.
- //
- SAFE_STRING_CONSTRAINT_CHECK ((String != NULL), RETURN_INVALID_PARAMETER);
- SAFE_STRING_CONSTRAINT_CHECK ((Data != NULL), RETURN_INVALID_PARAMETER);
-
- //
- // 2. The length of String shall not be greater than RSIZE_MAX.
- //
- if (RSIZE_MAX != 0) {
- SAFE_STRING_CONSTRAINT_CHECK ((StrnLenS (String, RSIZE_MAX + 1) <= RSIZE_MAX), RETURN_INVALID_PARAMETER);
- }
-
- if (EndPointer != NULL) {
- *EndPointer = (CHAR16 *) String;
- }
-
- //
- // Ignore the pad spaces (space or tab)
- //
- while ((*String == L' ') || (*String == L'\t')) {
- String++;
- }
-
- //
- // Ignore leading Zeros after the spaces
- //
- while (*String == L'0') {
- String++;
- }
-
- if (InternalCharToUpper (*String) == L'X') {
- if (*(String - 1) != L'0') {
- *Data = 0;
- return RETURN_SUCCESS;
- }
- //
- // Skip the 'X'
- //
- String++;
- }
-
- *Data = 0;
-
- while (InternalIsHexaDecimalDigitCharacter (*String)) {
- //
- // If the number represented by String overflows according to the range
- // defined by UINTN, then MAX_UINTN is stored in *Data and
- // RETURN_UNSUPPORTED is returned.
- //
- if (*Data > ((MAX_UINTN - InternalHexCharToUintn (*String)) >> 4)) {
- *Data = MAX_UINTN;
- if (EndPointer != NULL) {
- *EndPointer = (CHAR16 *) String;
- }
- return RETURN_UNSUPPORTED;
- }
-
- *Data = (*Data << 4) + InternalHexCharToUintn (*String);
- String++;
- }
-
- if (EndPointer != NULL) {
- *EndPointer = (CHAR16 *) String;
- }
- return RETURN_SUCCESS;
-}
-RETURN_STATUS
StrHexToUint64S (
CONST CHAR16 *String,
CHAR16 **EndPointer, OPTIONAL
@@ -1291,28 +1149,6 @@ StrHexToUint64 (
return Result;
}
-UINTN
-StrDecimalToUintn (
- CONST CHAR16 *String
- )
-{
- UINTN Result;
-
- StrDecimalToUintnS (String, (CHAR16 **) NULL, &Result);
- return Result;
-}
-
-UINTN
-StrHexToUintn (
- CONST CHAR16 *String
- )
-{
- UINTN Result;
-
- StrHexToUintnS (String, (CHAR16 **) NULL, &Result);
- return Result;
-}
-
UINTN
StrSize (
CONST CHAR16 *String
--
2.19.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v2 6/6] BaseTools/CommonLib: drop definition of MAX_UINTN
2018-11-30 22:45 [PATCH v2 0/6] BaseTools: get rid of MAX_UINTN Ard Biesheuvel
` (4 preceding siblings ...)
2018-11-30 22:45 ` [PATCH v2 5/6] BaseTools/CommonLib: get rid of 'native' type string parsing routines Ard Biesheuvel
@ 2018-11-30 22:45 ` Ard Biesheuvel
2018-12-03 10:28 ` Philippe Mathieu-Daudé
2018-12-03 13:08 ` Laszlo Ersek
2018-12-05 0:04 ` [PATCH v2 0/6] BaseTools: get rid " Gao, Liming
6 siblings, 2 replies; 28+ messages in thread
From: Ard Biesheuvel @ 2018-11-30 22:45 UTC (permalink / raw)
To: edk2-devel
Cc: Ard Biesheuvel, Laszlo Ersek, Yonghong Zhu, Liming Gao, Bob Feng,
Jaben Carsey
The maximum value that can be represented by the native word size
of the *target* should be irrelevant when compiling tools that
run on the build *host*. So drop the definition of MAX_UINTN, now
that we no longer use it.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Jaben Carsey <jaben.carsey@intel.com>
---
BaseTools/Source/C/Common/CommonLib.h | 1 -
1 file changed, 1 deletion(-)
diff --git a/BaseTools/Source/C/Common/CommonLib.h b/BaseTools/Source/C/Common/CommonLib.h
index 6930d9227b87..b1c6c00a3478 100644
--- a/BaseTools/Source/C/Common/CommonLib.h
+++ b/BaseTools/Source/C/Common/CommonLib.h
@@ -22,7 +22,6 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
#define MAX_LONG_FILE_PATH 500
-#define MAX_UINTN MAX_ADDRESS
#define MAX_UINT64 ((UINT64)0xFFFFFFFFFFFFFFFFULL)
#define MAX_UINT16 ((UINT16)0xFFFF)
#define MAX_UINT8 ((UINT8)0xFF)
--
2.19.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH v2 1/6] BaseTools/CommonLib: avoid using 'native' word size in IP address handling
2018-11-30 22:45 ` [PATCH v2 1/6] BaseTools/CommonLib: avoid using 'native' word size in IP address handling Ard Biesheuvel
@ 2018-12-03 10:25 ` Philippe Mathieu-Daudé
2018-12-03 12:50 ` Laszlo Ersek
1 sibling, 0 replies; 28+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-12-03 10:25 UTC (permalink / raw)
To: Ard Biesheuvel, edk2-devel; +Cc: Liming Gao, Jaben Carsey, Laszlo Ersek
On 30/11/18 23:45, Ard Biesheuvel wrote:
> In the context of the BaseTools, there is no such thing as a native word
> size, given that the same set of tools may be used to build a firmware
> image consisting of both 32-bit and 64-bit modules.
>
> So update StrToIpv4Address() and StrToIpv6Address() to use UINT64
> types instead of UINTN types when parsing strings.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> BaseTools/Source/C/Common/CommonLib.c | 28 ++++++++++----------
> 1 file changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/BaseTools/Source/C/Common/CommonLib.c b/BaseTools/Source/C/Common/CommonLib.c
> index 618aadac781a..7c559bc3c222 100644
> --- a/BaseTools/Source/C/Common/CommonLib.c
> +++ b/BaseTools/Source/C/Common/CommonLib.c
> @@ -1785,7 +1785,7 @@ StrToIpv4Address (
> {
> RETURN_STATUS Status;
> UINTN AddressIndex;
> - UINTN Uintn;
> + UINT64 Uint64;
> EFI_IPv4_ADDRESS LocalAddress;
> UINT8 LocalPrefixLength;
> CHAR16 *Pointer;
> @@ -1812,7 +1812,7 @@ StrToIpv4Address (
> //
> // Get D or P.
> //
> - Status = StrDecimalToUintnS ((CONST CHAR16 *) Pointer, &Pointer, &Uintn);
> + Status = StrDecimalToUint64S ((CONST CHAR16 *) Pointer, &Pointer, &Uint64);
> if (RETURN_ERROR (Status)) {
> return RETURN_UNSUPPORTED;
> }
> @@ -1820,18 +1820,18 @@ StrToIpv4Address (
> //
> // It's P.
> //
> - if (Uintn > 32) {
> + if (Uint64 > 32) {
> return RETURN_UNSUPPORTED;
> }
> - LocalPrefixLength = (UINT8) Uintn;
> + LocalPrefixLength = (UINT8) Uint64;
> } else {
> //
> // It's D.
> //
> - if (Uintn > MAX_UINT8) {
> + if (Uint64 > MAX_UINT8) {
> return RETURN_UNSUPPORTED;
> }
> - LocalAddress.Addr[AddressIndex] = (UINT8) Uintn;
> + LocalAddress.Addr[AddressIndex] = (UINT8) Uint64;
> AddressIndex++;
> }
>
> @@ -1888,7 +1888,7 @@ StrToIpv6Address (
> {
> RETURN_STATUS Status;
> UINTN AddressIndex;
> - UINTN Uintn;
> + UINT64 Uint64;
> EFI_IPv6_ADDRESS LocalAddress;
> UINT8 LocalPrefixLength;
> CONST CHAR16 *Pointer;
> @@ -1969,7 +1969,7 @@ StrToIpv6Address (
> //
> // Get X.
> //
> - Status = StrHexToUintnS (Pointer, &End, &Uintn);
> + Status = StrHexToUint64S (Pointer, &End, &Uint64);
> if (RETURN_ERROR (Status) || End - Pointer > 4) {
> //
> // Number of hexadecimal digit characters is no more than 4.
> @@ -1978,24 +1978,24 @@ StrToIpv6Address (
> }
> Pointer = End;
> //
> - // Uintn won't exceed MAX_UINT16 if number of hexadecimal digit characters is no more than 4.
> + // Uint64 won't exceed MAX_UINT16 if number of hexadecimal digit characters is no more than 4.
> //
> ASSERT (AddressIndex + 1 < ARRAY_SIZE (Address->Addr));
> - LocalAddress.Addr[AddressIndex] = (UINT8) ((UINT16) Uintn >> 8);
> - LocalAddress.Addr[AddressIndex + 1] = (UINT8) Uintn;
> + LocalAddress.Addr[AddressIndex] = (UINT8) ((UINT16) Uint64 >> 8);
> + LocalAddress.Addr[AddressIndex + 1] = (UINT8) Uint64;
> AddressIndex += 2;
> } else {
> //
> // Get P, then exit the loop.
> //
> - Status = StrDecimalToUintnS (Pointer, &End, &Uintn);
> - if (RETURN_ERROR (Status) || End == Pointer || Uintn > 128) {
> + Status = StrDecimalToUint64S (Pointer, &End, &Uint64);
> + if (RETURN_ERROR (Status) || End == Pointer || Uint64 > 128) {
> //
> // Prefix length should not exceed 128.
> //
> return RETURN_UNSUPPORTED;
> }
> - LocalPrefixLength = (UINT8) Uintn;
> + LocalPrefixLength = (UINT8) Uint64;
> Pointer = End;
> break;
> }
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 2/6] BaseTools/CommonLib: use explicit 64-bit type in Strtoi()
2018-11-30 22:45 ` [PATCH v2 2/6] BaseTools/CommonLib: use explicit 64-bit type in Strtoi() Ard Biesheuvel
@ 2018-12-03 10:25 ` Philippe Mathieu-Daudé
2018-12-03 12:52 ` Laszlo Ersek
1 sibling, 0 replies; 28+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-12-03 10:25 UTC (permalink / raw)
To: Ard Biesheuvel, edk2-devel; +Cc: Liming Gao, Jaben Carsey, Laszlo Ersek
On 30/11/18 23:45, Ard Biesheuvel wrote:
> Don't use the native word size string to number parsing routines,
> but instead, use the 64-bit one and cast to UINTN.
>
> Currently, the only user is in Source/C/DevicePath/DevicePathFromText.c
> which takes care to use Strtoi64 () unless it assumes the value fits
> in 32-bit, so this change is a no-op even on 32-bit build hosts.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Reviewed-by: Jaben Carsey <jaben.carsey@intel.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> BaseTools/Source/C/Common/CommonLib.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/BaseTools/Source/C/Common/CommonLib.c b/BaseTools/Source/C/Common/CommonLib.c
> index 7c559bc3c222..4a28f635f3a8 100644
> --- a/BaseTools/Source/C/Common/CommonLib.c
> +++ b/BaseTools/Source/C/Common/CommonLib.c
> @@ -2252,9 +2252,9 @@ Strtoi (
> )
> {
> if (IsHexStr (Str)) {
> - return StrHexToUintn (Str);
> + return (UINTN)StrHexToUint64 (Str);
> } else {
> - return StrDecimalToUintn (Str);
> + return (UINTN)StrDecimalToUint64 (Str);
> }
> }
>
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 3/6] BaseTools/DevicePath: use explicit 64-bit number parsing routines
2018-11-30 22:45 ` [PATCH v2 3/6] BaseTools/DevicePath: use explicit 64-bit number parsing routines Ard Biesheuvel
@ 2018-12-03 10:25 ` Philippe Mathieu-Daudé
2018-12-03 12:55 ` Laszlo Ersek
1 sibling, 0 replies; 28+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-12-03 10:25 UTC (permalink / raw)
To: Ard Biesheuvel, edk2-devel; +Cc: Liming Gao, Jaben Carsey, Laszlo Ersek
On 30/11/18 23:45, Ard Biesheuvel wrote:
> Replace invocations of StrHexToUintn() with StrHexToUint64(), so
> that we can drop the former.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Reviewed-by: Jaben Carsey <jaben.carsey@intel.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> BaseTools/Source/C/DevicePath/DevicePathFromText.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/BaseTools/Source/C/DevicePath/DevicePathFromText.c b/BaseTools/Source/C/DevicePath/DevicePathFromText.c
> index 555efa1acdde..6151926af9aa 100644
> --- a/BaseTools/Source/C/DevicePath/DevicePathFromText.c
> +++ b/BaseTools/Source/C/DevicePath/DevicePathFromText.c
> @@ -520,7 +520,7 @@ EisaIdFromText (
> return (((Text[0] - 'A' + 1) & 0x1f) << 10)
> + (((Text[1] - 'A' + 1) & 0x1f) << 5)
> + (((Text[2] - 'A' + 1) & 0x1f) << 0)
> - + (UINT32) (StrHexToUintn (&Text[3]) << 16)
> + + (UINT32) (StrHexToUint64 (&Text[3]) << 16)
> ;
> }
>
> @@ -1506,7 +1506,7 @@ DevPathFromTextNVMe (
>
> Index = sizeof (Nvme->NamespaceUuid) / sizeof (UINT8);
> while (Index-- != 0) {
> - Uuid[Index] = (UINT8) StrHexToUintn (SplitStr (&NamespaceUuidStr, L'-'));
> + Uuid[Index] = (UINT8) StrHexToUint64 (SplitStr (&NamespaceUuidStr, L'-'));
> }
>
> return (EFI_DEVICE_PATH_PROTOCOL *) Nvme;
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 5/6] BaseTools/CommonLib: get rid of 'native' type string parsing routines
2018-11-30 22:45 ` [PATCH v2 5/6] BaseTools/CommonLib: get rid of 'native' type string parsing routines Ard Biesheuvel
@ 2018-12-03 10:27 ` Philippe Mathieu-Daudé
2018-12-03 13:08 ` Laszlo Ersek
1 sibling, 0 replies; 28+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-12-03 10:27 UTC (permalink / raw)
To: Ard Biesheuvel, edk2-devel; +Cc: Liming Gao, Jaben Carsey, Laszlo Ersek
On 30/11/18 23:45, Ard Biesheuvel wrote:
> Parsing a string into an integer variable of the native word size
> is not defined for the BaseTools, since the same tools may be used
> to build firmware for different targets with different native word
> sizes.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> BaseTools/Source/C/Common/CommonLib.h | 24 ---
> BaseTools/Source/C/Common/CommonLib.c | 174 +-------------------
> 2 files changed, 5 insertions(+), 193 deletions(-)
>
> diff --git a/BaseTools/Source/C/Common/CommonLib.h b/BaseTools/Source/C/Common/CommonLib.h
> index fa10fac4682a..6930d9227b87 100644
> --- a/BaseTools/Source/C/Common/CommonLib.h
> +++ b/BaseTools/Source/C/Common/CommonLib.h
> @@ -250,16 +250,6 @@ StrSize (
> CONST CHAR16 *String
> );
>
> -UINTN
> -StrHexToUintn (
> - CONST CHAR16 *String
> - );
> -
> -UINTN
> -StrDecimalToUintn (
> - CONST CHAR16 *String
> - );
> -
> UINT64
> StrHexToUint64 (
> CONST CHAR16 *String
> @@ -277,13 +267,6 @@ StrHexToUint64S (
> UINT64 *Data
> );
>
> -RETURN_STATUS
> -StrHexToUintnS (
> - CONST CHAR16 *String,
> - CHAR16 **EndPointer, OPTIONAL
> - UINTN *Data
> - );
> -
> RETURN_STATUS
> StrDecimalToUint64S (
> CONST CHAR16 *String,
> @@ -291,13 +274,6 @@ StrDecimalToUint64S (
> UINT64 *Data
> );
>
> -RETURN_STATUS
> -StrDecimalToUintnS (
> - CONST CHAR16 *String,
> - CHAR16 **EndPointer, OPTIONAL
> - UINTN *Data
> - );
> -
> VOID *
> ReallocatePool (
> UINTN OldSize,
> diff --git a/BaseTools/Source/C/Common/CommonLib.c b/BaseTools/Source/C/Common/CommonLib.c
> index 4a28f635f3a8..42dfa821624d 100644
> --- a/BaseTools/Source/C/Common/CommonLib.c
> +++ b/BaseTools/Source/C/Common/CommonLib.c
> @@ -882,72 +882,6 @@ InternalSafeStringNoStrOverlap (
> return !InternalSafeStringIsOverlap (Str1, Size1 * sizeof(CHAR16), Str2, Size2 * sizeof(CHAR16));
> }
>
> -RETURN_STATUS
> -StrDecimalToUintnS (
> - CONST CHAR16 *String,
> - CHAR16 **EndPointer, OPTIONAL
> - UINTN *Data
> - )
> -{
> - ASSERT (((UINTN) String & BIT0) == 0);
> -
> - //
> - // 1. Neither String nor Data shall be a null pointer.
> - //
> - SAFE_STRING_CONSTRAINT_CHECK ((String != NULL), RETURN_INVALID_PARAMETER);
> - SAFE_STRING_CONSTRAINT_CHECK ((Data != NULL), RETURN_INVALID_PARAMETER);
> -
> - //
> - // 2. The length of String shall not be greater than RSIZE_MAX.
> - //
> - if (RSIZE_MAX != 0) {
> - SAFE_STRING_CONSTRAINT_CHECK ((StrnLenS (String, RSIZE_MAX + 1) <= RSIZE_MAX), RETURN_INVALID_PARAMETER);
> - }
> -
> - if (EndPointer != NULL) {
> - *EndPointer = (CHAR16 *) String;
> - }
> -
> - //
> - // Ignore the pad spaces (space or tab)
> - //
> - while ((*String == L' ') || (*String == L'\t')) {
> - String++;
> - }
> -
> - //
> - // Ignore leading Zeros after the spaces
> - //
> - while (*String == L'0') {
> - String++;
> - }
> -
> - *Data = 0;
> -
> - while (InternalIsDecimalDigitCharacter (*String)) {
> - //
> - // If the number represented by String overflows according to the range
> - // defined by UINTN, then MAX_UINTN is stored in *Data and
> - // RETURN_UNSUPPORTED is returned.
> - //
> - if (*Data > ((MAX_UINTN - (*String - L'0')) / 10)) {
> - *Data = MAX_UINTN;
> - if (EndPointer != NULL) {
> - *EndPointer = (CHAR16 *) String;
> - }
> - return RETURN_UNSUPPORTED;
> - }
> -
> - *Data = *Data * 10 + (*String - L'0');
> - String++;
> - }
> -
> - if (EndPointer != NULL) {
> - *EndPointer = (CHAR16 *) String;
> - }
> - return RETURN_SUCCESS;
> -}
> -
> /**
> Convert a Null-terminated Unicode decimal string to a value of type UINT64.
>
> @@ -1064,9 +998,9 @@ StrDecimalToUint64S (
>
> /**
> Convert a Null-terminated Unicode hexadecimal string to a value of type
> - UINTN.
> + UINT64.
>
> - This function outputs a value of type UINTN by interpreting the contents of
> + This function outputs a value of type UINT64 by interpreting the contents of
> the Unicode string specified by String as a hexadecimal number. The format of
> the input Unicode string String is:
>
> @@ -1091,8 +1025,8 @@ StrDecimalToUint64S (
>
> If String has no valid hexadecimal digits in the above format, then 0 is
> stored at the location pointed to by Data.
> - If the number represented by String exceeds the range defined by UINTN, then
> - MAX_UINTN is stored at the location pointed to by Data.
> + If the number represented by String exceeds the range defined by UINT64, then
> + MAX_UINT64 is stored at the location pointed to by Data.
>
> If EndPointer is not NULL, a pointer to the character that stopped the scan
> is stored at the location pointed to by EndPointer. If String has no valid
> @@ -1112,86 +1046,10 @@ StrDecimalToUint64S (
> characters, not including the
> Null-terminator.
> @retval RETURN_UNSUPPORTED If the number represented by String exceeds
> - the range defined by UINTN.
> + the range defined by UINT64.
>
> **/
> RETURN_STATUS
> -StrHexToUintnS (
> - CONST CHAR16 *String,
> - CHAR16 **EndPointer, OPTIONAL
> - UINTN *Data
> - )
> -{
> - ASSERT (((UINTN) String & BIT0) == 0);
> -
> - //
> - // 1. Neither String nor Data shall be a null pointer.
> - //
> - SAFE_STRING_CONSTRAINT_CHECK ((String != NULL), RETURN_INVALID_PARAMETER);
> - SAFE_STRING_CONSTRAINT_CHECK ((Data != NULL), RETURN_INVALID_PARAMETER);
> -
> - //
> - // 2. The length of String shall not be greater than RSIZE_MAX.
> - //
> - if (RSIZE_MAX != 0) {
> - SAFE_STRING_CONSTRAINT_CHECK ((StrnLenS (String, RSIZE_MAX + 1) <= RSIZE_MAX), RETURN_INVALID_PARAMETER);
> - }
> -
> - if (EndPointer != NULL) {
> - *EndPointer = (CHAR16 *) String;
> - }
> -
> - //
> - // Ignore the pad spaces (space or tab)
> - //
> - while ((*String == L' ') || (*String == L'\t')) {
> - String++;
> - }
> -
> - //
> - // Ignore leading Zeros after the spaces
> - //
> - while (*String == L'0') {
> - String++;
> - }
> -
> - if (InternalCharToUpper (*String) == L'X') {
> - if (*(String - 1) != L'0') {
> - *Data = 0;
> - return RETURN_SUCCESS;
> - }
> - //
> - // Skip the 'X'
> - //
> - String++;
> - }
> -
> - *Data = 0;
> -
> - while (InternalIsHexaDecimalDigitCharacter (*String)) {
> - //
> - // If the number represented by String overflows according to the range
> - // defined by UINTN, then MAX_UINTN is stored in *Data and
> - // RETURN_UNSUPPORTED is returned.
> - //
> - if (*Data > ((MAX_UINTN - InternalHexCharToUintn (*String)) >> 4)) {
> - *Data = MAX_UINTN;
> - if (EndPointer != NULL) {
> - *EndPointer = (CHAR16 *) String;
> - }
> - return RETURN_UNSUPPORTED;
> - }
> -
> - *Data = (*Data << 4) + InternalHexCharToUintn (*String);
> - String++;
> - }
> -
> - if (EndPointer != NULL) {
> - *EndPointer = (CHAR16 *) String;
> - }
> - return RETURN_SUCCESS;
> -}
> -RETURN_STATUS
> StrHexToUint64S (
> CONST CHAR16 *String,
> CHAR16 **EndPointer, OPTIONAL
> @@ -1291,28 +1149,6 @@ StrHexToUint64 (
> return Result;
> }
>
> -UINTN
> -StrDecimalToUintn (
> - CONST CHAR16 *String
> - )
> -{
> - UINTN Result;
> -
> - StrDecimalToUintnS (String, (CHAR16 **) NULL, &Result);
> - return Result;
> -}
> -
> -UINTN
> -StrHexToUintn (
> - CONST CHAR16 *String
> - )
> -{
> - UINTN Result;
> -
> - StrHexToUintnS (String, (CHAR16 **) NULL, &Result);
> - return Result;
> -}
> -
> UINTN
> StrSize (
> CONST CHAR16 *String
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 6/6] BaseTools/CommonLib: drop definition of MAX_UINTN
2018-11-30 22:45 ` [PATCH v2 6/6] BaseTools/CommonLib: drop definition of MAX_UINTN Ard Biesheuvel
@ 2018-12-03 10:28 ` Philippe Mathieu-Daudé
2018-12-03 13:08 ` Laszlo Ersek
1 sibling, 0 replies; 28+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-12-03 10:28 UTC (permalink / raw)
To: Ard Biesheuvel, edk2-devel; +Cc: Liming Gao, Jaben Carsey, Laszlo Ersek
On 30/11/18 23:45, Ard Biesheuvel wrote:
> The maximum value that can be represented by the native word size
> of the *target* should be irrelevant when compiling tools that
> run on the build *host*. So drop the definition of MAX_UINTN, now
> that we no longer use it.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Reviewed-by: Jaben Carsey <jaben.carsey@intel.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> BaseTools/Source/C/Common/CommonLib.h | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/BaseTools/Source/C/Common/CommonLib.h b/BaseTools/Source/C/Common/CommonLib.h
> index 6930d9227b87..b1c6c00a3478 100644
> --- a/BaseTools/Source/C/Common/CommonLib.h
> +++ b/BaseTools/Source/C/Common/CommonLib.h
> @@ -22,7 +22,6 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
>
> #define MAX_LONG_FILE_PATH 500
>
> -#define MAX_UINTN MAX_ADDRESS
> #define MAX_UINT64 ((UINT64)0xFFFFFFFFFFFFFFFFULL)
> #define MAX_UINT16 ((UINT16)0xFFFF)
> #define MAX_UINT8 ((UINT8)0xFF)
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 1/6] BaseTools/CommonLib: avoid using 'native' word size in IP address handling
2018-11-30 22:45 ` [PATCH v2 1/6] BaseTools/CommonLib: avoid using 'native' word size in IP address handling Ard Biesheuvel
2018-12-03 10:25 ` Philippe Mathieu-Daudé
@ 2018-12-03 12:50 ` Laszlo Ersek
1 sibling, 0 replies; 28+ messages in thread
From: Laszlo Ersek @ 2018-12-03 12:50 UTC (permalink / raw)
To: Ard Biesheuvel, edk2-devel
Cc: Yonghong Zhu, Liming Gao, Bob Feng, Jaben Carsey
On 11/30/18 23:45, Ard Biesheuvel wrote:
> In the context of the BaseTools, there is no such thing as a native word
> size, given that the same set of tools may be used to build a firmware
> image consisting of both 32-bit and 64-bit modules.
>
> So update StrToIpv4Address() and StrToIpv6Address() to use UINT64
> types instead of UINTN types when parsing strings.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> BaseTools/Source/C/Common/CommonLib.c | 28 ++++++++++----------
> 1 file changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/BaseTools/Source/C/Common/CommonLib.c b/BaseTools/Source/C/Common/CommonLib.c
> index 618aadac781a..7c559bc3c222 100644
> --- a/BaseTools/Source/C/Common/CommonLib.c
> +++ b/BaseTools/Source/C/Common/CommonLib.c
> @@ -1785,7 +1785,7 @@ StrToIpv4Address (
> {
> RETURN_STATUS Status;
> UINTN AddressIndex;
> - UINTN Uintn;
> + UINT64 Uint64;
> EFI_IPv4_ADDRESS LocalAddress;
> UINT8 LocalPrefixLength;
> CHAR16 *Pointer;
> @@ -1812,7 +1812,7 @@ StrToIpv4Address (
> //
> // Get D or P.
> //
> - Status = StrDecimalToUintnS ((CONST CHAR16 *) Pointer, &Pointer, &Uintn);
> + Status = StrDecimalToUint64S ((CONST CHAR16 *) Pointer, &Pointer, &Uint64);
> if (RETURN_ERROR (Status)) {
> return RETURN_UNSUPPORTED;
> }
> @@ -1820,18 +1820,18 @@ StrToIpv4Address (
> //
> // It's P.
> //
> - if (Uintn > 32) {
> + if (Uint64 > 32) {
> return RETURN_UNSUPPORTED;
> }
> - LocalPrefixLength = (UINT8) Uintn;
> + LocalPrefixLength = (UINT8) Uint64;
> } else {
> //
> // It's D.
> //
> - if (Uintn > MAX_UINT8) {
> + if (Uint64 > MAX_UINT8) {
> return RETURN_UNSUPPORTED;
> }
> - LocalAddress.Addr[AddressIndex] = (UINT8) Uintn;
> + LocalAddress.Addr[AddressIndex] = (UINT8) Uint64;
> AddressIndex++;
> }
>
> @@ -1888,7 +1888,7 @@ StrToIpv6Address (
> {
> RETURN_STATUS Status;
> UINTN AddressIndex;
> - UINTN Uintn;
> + UINT64 Uint64;
> EFI_IPv6_ADDRESS LocalAddress;
> UINT8 LocalPrefixLength;
> CONST CHAR16 *Pointer;
> @@ -1969,7 +1969,7 @@ StrToIpv6Address (
> //
> // Get X.
> //
> - Status = StrHexToUintnS (Pointer, &End, &Uintn);
> + Status = StrHexToUint64S (Pointer, &End, &Uint64);
> if (RETURN_ERROR (Status) || End - Pointer > 4) {
> //
> // Number of hexadecimal digit characters is no more than 4.
> @@ -1978,24 +1978,24 @@ StrToIpv6Address (
> }
> Pointer = End;
> //
> - // Uintn won't exceed MAX_UINT16 if number of hexadecimal digit characters is no more than 4.
> + // Uint64 won't exceed MAX_UINT16 if number of hexadecimal digit characters is no more than 4.
> //
> ASSERT (AddressIndex + 1 < ARRAY_SIZE (Address->Addr));
> - LocalAddress.Addr[AddressIndex] = (UINT8) ((UINT16) Uintn >> 8);
> - LocalAddress.Addr[AddressIndex + 1] = (UINT8) Uintn;
> + LocalAddress.Addr[AddressIndex] = (UINT8) ((UINT16) Uint64 >> 8);
> + LocalAddress.Addr[AddressIndex + 1] = (UINT8) Uint64;
> AddressIndex += 2;
> } else {
> //
> // Get P, then exit the loop.
> //
> - Status = StrDecimalToUintnS (Pointer, &End, &Uintn);
> - if (RETURN_ERROR (Status) || End == Pointer || Uintn > 128) {
> + Status = StrDecimalToUint64S (Pointer, &End, &Uint64);
> + if (RETURN_ERROR (Status) || End == Pointer || Uint64 > 128) {
> //
> // Prefix length should not exceed 128.
> //
> return RETURN_UNSUPPORTED;
> }
> - LocalPrefixLength = (UINT8) Uintn;
> + LocalPrefixLength = (UINT8) Uint64;
> Pointer = End;
> break;
> }
>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 2/6] BaseTools/CommonLib: use explicit 64-bit type in Strtoi()
2018-11-30 22:45 ` [PATCH v2 2/6] BaseTools/CommonLib: use explicit 64-bit type in Strtoi() Ard Biesheuvel
2018-12-03 10:25 ` Philippe Mathieu-Daudé
@ 2018-12-03 12:52 ` Laszlo Ersek
1 sibling, 0 replies; 28+ messages in thread
From: Laszlo Ersek @ 2018-12-03 12:52 UTC (permalink / raw)
To: Ard Biesheuvel, edk2-devel
Cc: Yonghong Zhu, Liming Gao, Bob Feng, Jaben Carsey
On 11/30/18 23:45, Ard Biesheuvel wrote:
> Don't use the native word size string to number parsing routines,
> but instead, use the 64-bit one and cast to UINTN.
>
> Currently, the only user is in Source/C/DevicePath/DevicePathFromText.c
> which takes care to use Strtoi64 () unless it assumes the value fits
> in 32-bit, so this change is a no-op even on 32-bit build hosts.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Reviewed-by: Jaben Carsey <jaben.carsey@intel.com>
> ---
> BaseTools/Source/C/Common/CommonLib.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/BaseTools/Source/C/Common/CommonLib.c b/BaseTools/Source/C/Common/CommonLib.c
> index 7c559bc3c222..4a28f635f3a8 100644
> --- a/BaseTools/Source/C/Common/CommonLib.c
> +++ b/BaseTools/Source/C/Common/CommonLib.c
> @@ -2252,9 +2252,9 @@ Strtoi (
> )
> {
> if (IsHexStr (Str)) {
> - return StrHexToUintn (Str);
> + return (UINTN)StrHexToUint64 (Str);
> } else {
> - return StrDecimalToUintn (Str);
> + return (UINTN)StrDecimalToUint64 (Str);
> }
> }
>
>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 3/6] BaseTools/DevicePath: use explicit 64-bit number parsing routines
2018-11-30 22:45 ` [PATCH v2 3/6] BaseTools/DevicePath: use explicit 64-bit number parsing routines Ard Biesheuvel
2018-12-03 10:25 ` Philippe Mathieu-Daudé
@ 2018-12-03 12:55 ` Laszlo Ersek
1 sibling, 0 replies; 28+ messages in thread
From: Laszlo Ersek @ 2018-12-03 12:55 UTC (permalink / raw)
To: Ard Biesheuvel, edk2-devel
Cc: Yonghong Zhu, Liming Gao, Bob Feng, Jaben Carsey
On 11/30/18 23:45, Ard Biesheuvel wrote:
> Replace invocations of StrHexToUintn() with StrHexToUint64(), so
> that we can drop the former.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Reviewed-by: Jaben Carsey <jaben.carsey@intel.com>
> ---
> BaseTools/Source/C/DevicePath/DevicePathFromText.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/BaseTools/Source/C/DevicePath/DevicePathFromText.c b/BaseTools/Source/C/DevicePath/DevicePathFromText.c
> index 555efa1acdde..6151926af9aa 100644
> --- a/BaseTools/Source/C/DevicePath/DevicePathFromText.c
> +++ b/BaseTools/Source/C/DevicePath/DevicePathFromText.c
> @@ -520,7 +520,7 @@ EisaIdFromText (
> return (((Text[0] - 'A' + 1) & 0x1f) << 10)
> + (((Text[1] - 'A' + 1) & 0x1f) << 5)
> + (((Text[2] - 'A' + 1) & 0x1f) << 0)
> - + (UINT32) (StrHexToUintn (&Text[3]) << 16)
> + + (UINT32) (StrHexToUint64 (&Text[3]) << 16)
> ;
> }
>
This (theoretically) introduces a bit-shift on a UINT64 value, which
might result in a compiler intrinsic call on 32-bit build hosts. But
that's fine, because this is not firmware code, but hosted code.
> @@ -1506,7 +1506,7 @@ DevPathFromTextNVMe (
>
> Index = sizeof (Nvme->NamespaceUuid) / sizeof (UINT8);
> while (Index-- != 0) {
> - Uuid[Index] = (UINT8) StrHexToUintn (SplitStr (&NamespaceUuidStr, L'-'));
> + Uuid[Index] = (UINT8) StrHexToUint64 (SplitStr (&NamespaceUuidStr, L'-'));
> }
>
> return (EFI_DEVICE_PATH_PROTOCOL *) Nvme;
>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Thanks
Laszlo
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 4/6] BaseTools/DevicePath: use MAX_UINT16 as default device path max size
2018-11-30 22:45 ` [PATCH v2 4/6] BaseTools/DevicePath: use MAX_UINT16 as default device path max size Ard Biesheuvel
@ 2018-12-03 13:05 ` Laszlo Ersek
2018-12-05 0:04 ` Gao, Liming
0 siblings, 1 reply; 28+ messages in thread
From: Laszlo Ersek @ 2018-12-03 13:05 UTC (permalink / raw)
To: Ard Biesheuvel, edk2-devel
Cc: Yonghong Zhu, Liming Gao, Bob Feng, Jaben Carsey
On 11/30/18 23:45, Ard Biesheuvel wrote:
> Replace the default size limit of IsDevicePathValid() with a value
> that does not depend on the native word size of the build host.
>
> 64 KB seems sufficient as the upper bound of a device path handled
> by UEFI.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Reviewed-by: Jaben Carsey <jaben.carsey@intel.com>
> ---
> BaseTools/Source/C/DevicePath/DevicePathUtilities.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/BaseTools/Source/C/DevicePath/DevicePathUtilities.c b/BaseTools/Source/C/DevicePath/DevicePathUtilities.c
> index d4ec2742b7c8..ba7f83e53070 100644
> --- a/BaseTools/Source/C/DevicePath/DevicePathUtilities.c
> +++ b/BaseTools/Source/C/DevicePath/DevicePathUtilities.c
> @@ -62,7 +62,7 @@ IsDevicePathValid (
> ASSERT (DevicePath != NULL);
>
> if (MaxSize == 0) {
> - MaxSize = MAX_UINTN;
> + MaxSize = MAX_UINT16;
> }
>
> //
> @@ -78,7 +78,7 @@ IsDevicePathValid (
> return FALSE;
> }
>
> - if (NodeLength > MAX_UINTN - Size) {
> + if (NodeLength > MAX_UINT16 - Size) {
> return FALSE;
> }
> Size += NodeLength;
>
I'm somewhat undecided about this patch.
(1) IsDevicePathValid() also exists in:
- MdePkg/Library/UefiDevicePathLib/DevicePathUtilities.c
- MdePkg/Library/UefiDevicePathLibDevicePathProtocol/UefiDevicePathLib.c
Both have:
if (MaxSize == 0) {
MaxSize = MAX_UINTN;
}
Relative to those, this change departs quite strongly.
(2) In addition, a single device path node may extend up to 64KB. That
would be pathologic, yes, but the option is there.
... Of course, we are discussing theoretical limits. Still I'd feel more
comfortable with MAX_UINT32. Lifting the limit from 64K to 4G wouldn't
cost us anything (in development effort), it would be a no-op on 32-bit
build hosts, it would be a theoretical-only change on 64-bit build
hosts, and it would leave us with a larger "safety margin".
I won't insist, but I thought I should raise this. (Sorry if this has
been discussed under v1 already.) If you agree, no need to repost (from
my side anyway) just for this.
With or without the update:
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Thanks
Laszlo
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 5/6] BaseTools/CommonLib: get rid of 'native' type string parsing routines
2018-11-30 22:45 ` [PATCH v2 5/6] BaseTools/CommonLib: get rid of 'native' type string parsing routines Ard Biesheuvel
2018-12-03 10:27 ` Philippe Mathieu-Daudé
@ 2018-12-03 13:08 ` Laszlo Ersek
1 sibling, 0 replies; 28+ messages in thread
From: Laszlo Ersek @ 2018-12-03 13:08 UTC (permalink / raw)
To: Ard Biesheuvel, edk2-devel
Cc: Yonghong Zhu, Liming Gao, Bob Feng, Jaben Carsey
On 11/30/18 23:45, Ard Biesheuvel wrote:
> Parsing a string into an integer variable of the native word size
> is not defined for the BaseTools, since the same tools may be used
> to build firmware for different targets with different native word
> sizes.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> BaseTools/Source/C/Common/CommonLib.h | 24 ---
> BaseTools/Source/C/Common/CommonLib.c | 174 +-------------------
> 2 files changed, 5 insertions(+), 193 deletions(-)
>
> diff --git a/BaseTools/Source/C/Common/CommonLib.h b/BaseTools/Source/C/Common/CommonLib.h
> index fa10fac4682a..6930d9227b87 100644
> --- a/BaseTools/Source/C/Common/CommonLib.h
> +++ b/BaseTools/Source/C/Common/CommonLib.h
> @@ -250,16 +250,6 @@ StrSize (
> CONST CHAR16 *String
> );
>
> -UINTN
> -StrHexToUintn (
> - CONST CHAR16 *String
> - );
> -
> -UINTN
> -StrDecimalToUintn (
> - CONST CHAR16 *String
> - );
> -
> UINT64
> StrHexToUint64 (
> CONST CHAR16 *String
> @@ -277,13 +267,6 @@ StrHexToUint64S (
> UINT64 *Data
> );
>
> -RETURN_STATUS
> -StrHexToUintnS (
> - CONST CHAR16 *String,
> - CHAR16 **EndPointer, OPTIONAL
> - UINTN *Data
> - );
> -
> RETURN_STATUS
> StrDecimalToUint64S (
> CONST CHAR16 *String,
> @@ -291,13 +274,6 @@ StrDecimalToUint64S (
> UINT64 *Data
> );
>
> -RETURN_STATUS
> -StrDecimalToUintnS (
> - CONST CHAR16 *String,
> - CHAR16 **EndPointer, OPTIONAL
> - UINTN *Data
> - );
> -
> VOID *
> ReallocatePool (
> UINTN OldSize,
> diff --git a/BaseTools/Source/C/Common/CommonLib.c b/BaseTools/Source/C/Common/CommonLib.c
> index 4a28f635f3a8..42dfa821624d 100644
> --- a/BaseTools/Source/C/Common/CommonLib.c
> +++ b/BaseTools/Source/C/Common/CommonLib.c
> @@ -882,72 +882,6 @@ InternalSafeStringNoStrOverlap (
> return !InternalSafeStringIsOverlap (Str1, Size1 * sizeof(CHAR16), Str2, Size2 * sizeof(CHAR16));
> }
>
> -RETURN_STATUS
> -StrDecimalToUintnS (
> - CONST CHAR16 *String,
> - CHAR16 **EndPointer, OPTIONAL
> - UINTN *Data
> - )
> -{
> - ASSERT (((UINTN) String & BIT0) == 0);
> -
> - //
> - // 1. Neither String nor Data shall be a null pointer.
> - //
> - SAFE_STRING_CONSTRAINT_CHECK ((String != NULL), RETURN_INVALID_PARAMETER);
> - SAFE_STRING_CONSTRAINT_CHECK ((Data != NULL), RETURN_INVALID_PARAMETER);
> -
> - //
> - // 2. The length of String shall not be greater than RSIZE_MAX.
> - //
> - if (RSIZE_MAX != 0) {
> - SAFE_STRING_CONSTRAINT_CHECK ((StrnLenS (String, RSIZE_MAX + 1) <= RSIZE_MAX), RETURN_INVALID_PARAMETER);
> - }
> -
> - if (EndPointer != NULL) {
> - *EndPointer = (CHAR16 *) String;
> - }
> -
> - //
> - // Ignore the pad spaces (space or tab)
> - //
> - while ((*String == L' ') || (*String == L'\t')) {
> - String++;
> - }
> -
> - //
> - // Ignore leading Zeros after the spaces
> - //
> - while (*String == L'0') {
> - String++;
> - }
> -
> - *Data = 0;
> -
> - while (InternalIsDecimalDigitCharacter (*String)) {
> - //
> - // If the number represented by String overflows according to the range
> - // defined by UINTN, then MAX_UINTN is stored in *Data and
> - // RETURN_UNSUPPORTED is returned.
> - //
> - if (*Data > ((MAX_UINTN - (*String - L'0')) / 10)) {
> - *Data = MAX_UINTN;
> - if (EndPointer != NULL) {
> - *EndPointer = (CHAR16 *) String;
> - }
> - return RETURN_UNSUPPORTED;
> - }
> -
> - *Data = *Data * 10 + (*String - L'0');
> - String++;
> - }
> -
> - if (EndPointer != NULL) {
> - *EndPointer = (CHAR16 *) String;
> - }
> - return RETURN_SUCCESS;
> -}
> -
> /**
> Convert a Null-terminated Unicode decimal string to a value of type UINT64.
>
> @@ -1064,9 +998,9 @@ StrDecimalToUint64S (
>
> /**
> Convert a Null-terminated Unicode hexadecimal string to a value of type
> - UINTN.
> + UINT64.
>
> - This function outputs a value of type UINTN by interpreting the contents of
> + This function outputs a value of type UINT64 by interpreting the contents of
> the Unicode string specified by String as a hexadecimal number. The format of
> the input Unicode string String is:
>
> @@ -1091,8 +1025,8 @@ StrDecimalToUint64S (
>
> If String has no valid hexadecimal digits in the above format, then 0 is
> stored at the location pointed to by Data.
> - If the number represented by String exceeds the range defined by UINTN, then
> - MAX_UINTN is stored at the location pointed to by Data.
> + If the number represented by String exceeds the range defined by UINT64, then
> + MAX_UINT64 is stored at the location pointed to by Data.
>
> If EndPointer is not NULL, a pointer to the character that stopped the scan
> is stored at the location pointed to by EndPointer. If String has no valid
> @@ -1112,86 +1046,10 @@ StrDecimalToUint64S (
> characters, not including the
> Null-terminator.
> @retval RETURN_UNSUPPORTED If the number represented by String exceeds
> - the range defined by UINTN.
> + the range defined by UINT64.
>
> **/
> RETURN_STATUS
> -StrHexToUintnS (
> - CONST CHAR16 *String,
> - CHAR16 **EndPointer, OPTIONAL
> - UINTN *Data
> - )
> -{
> - ASSERT (((UINTN) String & BIT0) == 0);
> -
> - //
> - // 1. Neither String nor Data shall be a null pointer.
> - //
> - SAFE_STRING_CONSTRAINT_CHECK ((String != NULL), RETURN_INVALID_PARAMETER);
> - SAFE_STRING_CONSTRAINT_CHECK ((Data != NULL), RETURN_INVALID_PARAMETER);
> -
> - //
> - // 2. The length of String shall not be greater than RSIZE_MAX.
> - //
> - if (RSIZE_MAX != 0) {
> - SAFE_STRING_CONSTRAINT_CHECK ((StrnLenS (String, RSIZE_MAX + 1) <= RSIZE_MAX), RETURN_INVALID_PARAMETER);
> - }
> -
> - if (EndPointer != NULL) {
> - *EndPointer = (CHAR16 *) String;
> - }
> -
> - //
> - // Ignore the pad spaces (space or tab)
> - //
> - while ((*String == L' ') || (*String == L'\t')) {
> - String++;
> - }
> -
> - //
> - // Ignore leading Zeros after the spaces
> - //
> - while (*String == L'0') {
> - String++;
> - }
> -
> - if (InternalCharToUpper (*String) == L'X') {
> - if (*(String - 1) != L'0') {
> - *Data = 0;
> - return RETURN_SUCCESS;
> - }
> - //
> - // Skip the 'X'
> - //
> - String++;
> - }
> -
> - *Data = 0;
> -
> - while (InternalIsHexaDecimalDigitCharacter (*String)) {
> - //
> - // If the number represented by String overflows according to the range
> - // defined by UINTN, then MAX_UINTN is stored in *Data and
> - // RETURN_UNSUPPORTED is returned.
> - //
> - if (*Data > ((MAX_UINTN - InternalHexCharToUintn (*String)) >> 4)) {
> - *Data = MAX_UINTN;
> - if (EndPointer != NULL) {
> - *EndPointer = (CHAR16 *) String;
> - }
> - return RETURN_UNSUPPORTED;
> - }
> -
> - *Data = (*Data << 4) + InternalHexCharToUintn (*String);
> - String++;
> - }
> -
> - if (EndPointer != NULL) {
> - *EndPointer = (CHAR16 *) String;
> - }
> - return RETURN_SUCCESS;
> -}
> -RETURN_STATUS
> StrHexToUint64S (
> CONST CHAR16 *String,
> CHAR16 **EndPointer, OPTIONAL
> @@ -1291,28 +1149,6 @@ StrHexToUint64 (
> return Result;
> }
>
> -UINTN
> -StrDecimalToUintn (
> - CONST CHAR16 *String
> - )
> -{
> - UINTN Result;
> -
> - StrDecimalToUintnS (String, (CHAR16 **) NULL, &Result);
> - return Result;
> -}
> -
> -UINTN
> -StrHexToUintn (
> - CONST CHAR16 *String
> - )
> -{
> - UINTN Result;
> -
> - StrHexToUintnS (String, (CHAR16 **) NULL, &Result);
> - return Result;
> -}
> -
> UINTN
> StrSize (
> CONST CHAR16 *String
>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 6/6] BaseTools/CommonLib: drop definition of MAX_UINTN
2018-11-30 22:45 ` [PATCH v2 6/6] BaseTools/CommonLib: drop definition of MAX_UINTN Ard Biesheuvel
2018-12-03 10:28 ` Philippe Mathieu-Daudé
@ 2018-12-03 13:08 ` Laszlo Ersek
2018-12-11 7:11 ` David F.
1 sibling, 1 reply; 28+ messages in thread
From: Laszlo Ersek @ 2018-12-03 13:08 UTC (permalink / raw)
To: Ard Biesheuvel, edk2-devel
Cc: Yonghong Zhu, Liming Gao, Bob Feng, Jaben Carsey
On 11/30/18 23:45, Ard Biesheuvel wrote:
> The maximum value that can be represented by the native word size
> of the *target* should be irrelevant when compiling tools that
> run on the build *host*. So drop the definition of MAX_UINTN, now
> that we no longer use it.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Reviewed-by: Jaben Carsey <jaben.carsey@intel.com>
> ---
> BaseTools/Source/C/Common/CommonLib.h | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/BaseTools/Source/C/Common/CommonLib.h b/BaseTools/Source/C/Common/CommonLib.h
> index 6930d9227b87..b1c6c00a3478 100644
> --- a/BaseTools/Source/C/Common/CommonLib.h
> +++ b/BaseTools/Source/C/Common/CommonLib.h
> @@ -22,7 +22,6 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
>
> #define MAX_LONG_FILE_PATH 500
>
> -#define MAX_UINTN MAX_ADDRESS
> #define MAX_UINT64 ((UINT64)0xFFFFFFFFFFFFFFFFULL)
> #define MAX_UINT16 ((UINT16)0xFFFF)
> #define MAX_UINT8 ((UINT8)0xFF)
>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 4/6] BaseTools/DevicePath: use MAX_UINT16 as default device path max size
2018-12-03 13:05 ` Laszlo Ersek
@ 2018-12-05 0:04 ` Gao, Liming
2018-12-05 7:42 ` Ard Biesheuvel
0 siblings, 1 reply; 28+ messages in thread
From: Gao, Liming @ 2018-12-05 0:04 UTC (permalink / raw)
To: Laszlo Ersek, Ard Biesheuvel, edk2-devel@lists.01.org
Cc: Zhu, Yonghong, Feng, Bob C, Carsey, Jaben
Laszlo:
I agree with you. MAX_UINT32 is more comfortable.
Thanks
Liming
>-----Original Message-----
>From: Laszlo Ersek [mailto:lersek@redhat.com]
>Sent: Monday, December 03, 2018 9:06 PM
>To: Ard Biesheuvel <ard.biesheuvel@linaro.org>; edk2-devel@lists.01.org
>Cc: Zhu, Yonghong <yonghong.zhu@intel.com>; Gao, Liming
><liming.gao@intel.com>; Feng, Bob C <bob.c.feng@intel.com>; Carsey, Jaben
><jaben.carsey@intel.com>
>Subject: Re: [PATCH v2 4/6] BaseTools/DevicePath: use MAX_UINT16 as
>default device path max size
>
>On 11/30/18 23:45, Ard Biesheuvel wrote:
>> Replace the default size limit of IsDevicePathValid() with a value
>> that does not depend on the native word size of the build host.
>>
>> 64 KB seems sufficient as the upper bound of a device path handled
>> by UEFI.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> Reviewed-by: Jaben Carsey <jaben.carsey@intel.com>
>> ---
>> BaseTools/Source/C/DevicePath/DevicePathUtilities.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/BaseTools/Source/C/DevicePath/DevicePathUtilities.c
>b/BaseTools/Source/C/DevicePath/DevicePathUtilities.c
>> index d4ec2742b7c8..ba7f83e53070 100644
>> --- a/BaseTools/Source/C/DevicePath/DevicePathUtilities.c
>> +++ b/BaseTools/Source/C/DevicePath/DevicePathUtilities.c
>> @@ -62,7 +62,7 @@ IsDevicePathValid (
>> ASSERT (DevicePath != NULL);
>>
>> if (MaxSize == 0) {
>> - MaxSize = MAX_UINTN;
>> + MaxSize = MAX_UINT16;
>> }
>>
>> //
>> @@ -78,7 +78,7 @@ IsDevicePathValid (
>> return FALSE;
>> }
>>
>> - if (NodeLength > MAX_UINTN - Size) {
>> + if (NodeLength > MAX_UINT16 - Size) {
>> return FALSE;
>> }
>> Size += NodeLength;
>>
>
>I'm somewhat undecided about this patch.
>
>(1) IsDevicePathValid() also exists in:
>
>- MdePkg/Library/UefiDevicePathLib/DevicePathUtilities.c
>- MdePkg/Library/UefiDevicePathLibDevicePathProtocol/UefiDevicePathLib.c
>
>Both have:
>
> if (MaxSize == 0) {
> MaxSize = MAX_UINTN;
> }
>
>Relative to those, this change departs quite strongly.
>
>
>(2) In addition, a single device path node may extend up to 64KB. That
>would be pathologic, yes, but the option is there.
>
>
>... Of course, we are discussing theoretical limits. Still I'd feel more
>comfortable with MAX_UINT32. Lifting the limit from 64K to 4G wouldn't
>cost us anything (in development effort), it would be a no-op on 32-bit
>build hosts, it would be a theoretical-only change on 64-bit build
>hosts, and it would leave us with a larger "safety margin".
>
>I won't insist, but I thought I should raise this. (Sorry if this has
>been discussed under v1 already.) If you agree, no need to repost (from
>my side anyway) just for this.
>
>With or without the update:
>
>Reviewed-by: Laszlo Ersek <lersek@redhat.com>
>
>Thanks
>Laszlo
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 0/6] BaseTools: get rid of MAX_UINTN
2018-11-30 22:45 [PATCH v2 0/6] BaseTools: get rid of MAX_UINTN Ard Biesheuvel
` (5 preceding siblings ...)
2018-11-30 22:45 ` [PATCH v2 6/6] BaseTools/CommonLib: drop definition of MAX_UINTN Ard Biesheuvel
@ 2018-12-05 0:04 ` Gao, Liming
2018-12-05 8:12 ` Ard Biesheuvel
6 siblings, 1 reply; 28+ messages in thread
From: Gao, Liming @ 2018-12-05 0:04 UTC (permalink / raw)
To: Ard Biesheuvel, edk2-devel@lists.01.org; +Cc: Carsey, Jaben, Laszlo Ersek
Reviewed-by: Liming Gao <liming.gao@intel.com> for this serials.
On patch 4, I have the same comments to Laszlo.
>-----Original Message-----
>From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ard
>Biesheuvel
>Sent: Saturday, December 01, 2018 6:46 AM
>To: edk2-devel@lists.01.org
>Cc: Gao, Liming <liming.gao@intel.com>; Carsey, Jaben
><jaben.carsey@intel.com>; Laszlo Ersek <lersek@redhat.com>
>Subject: [edk2] [PATCH v2 0/6] BaseTools: get rid of MAX_UINTN
>
>There should be no reason for the build tools to care about the native
>word size of a particular target, so relying on a definition of MAX_UINTN
>is definitely wrong, and most likely inaccurate on 32-bit build hosts.
>
>So refactor the code in CommonLib and DevicePath so we no longer rely
>on this definition.
>
>Changes since v1:
>- miss type change in #1 causing a build failure on MSVC
>- add acks from Jaben
>
>Cc: Laszlo Ersek <lersek@redhat.com>
>Cc: Yonghong Zhu <yonghong.zhu@intel.com>
>Cc: Liming Gao <liming.gao@intel.com>
>Cc: Bob Feng <bob.c.feng@intel.com>
>Cc: Jaben Carsey <jaben.carsey@intel.com>
>
>Ard Biesheuvel (6):
> BaseTools/CommonLib: avoid using 'native' word size in IP address
> handling
> BaseTools/CommonLib: use explicit 64-bit type in Strtoi()
> BaseTools/DevicePath: use explicit 64-bit number parsing routines
> BaseTools/DevicePath: use MAX_UINT16 as default device path max size
> BaseTools/CommonLib: get rid of 'native' type string parsing routines
> BaseTools/CommonLib: drop definition of MAX_UINTN
>
> BaseTools/Source/C/Common/CommonLib.h | 25 ---
> BaseTools/Source/C/Common/CommonLib.c | 206 ++----------------
> .../Source/C/DevicePath/DevicePathFromText.c | 4 +-
> .../Source/C/DevicePath/DevicePathUtilities.c | 4 +-
> 4 files changed, 25 insertions(+), 214 deletions(-)
>
>--
>2.19.1
>
>_______________________________________________
>edk2-devel mailing list
>edk2-devel@lists.01.org
>https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 4/6] BaseTools/DevicePath: use MAX_UINT16 as default device path max size
2018-12-05 0:04 ` Gao, Liming
@ 2018-12-05 7:42 ` Ard Biesheuvel
2018-12-05 7:53 ` Gao, Liming
0 siblings, 1 reply; 28+ messages in thread
From: Ard Biesheuvel @ 2018-12-05 7:42 UTC (permalink / raw)
To: Gao, Liming
Cc: Laszlo Ersek, edk2-devel@lists.01.org, Zhu, Yonghong, Feng, Bob C,
Carsey, Jaben
On Wed, 5 Dec 2018 at 01:04, Gao, Liming <liming.gao@intel.com> wrote:
>
> Laszlo:
> I agree with you. MAX_UINT32 is more comfortable.
>
Liming,
No definitions for MAX_UINT32 exist currently in BaseTools, so I will
have to add the following:
diff --git a/BaseTools/Source/C/Common/CommonLib.h
b/BaseTools/Source/C/Common/CommonLib.h
index b1c6c00a3478..1c40180329c4 100644
--- a/BaseTools/Source/C/Common/CommonLib.h
+++ b/BaseTools/Source/C/Common/CommonLib.h
@@ -23,6 +23,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND,
EITHER EXPRESS OR IMPLIED.
#define MAX_LONG_FILE_PATH 500
#define MAX_UINT64 ((UINT64)0xFFFFFFFFFFFFFFFFULL)
+#define MAX_UINT32 ((UINT32)0xFFFFFFFFULL)
#define MAX_UINT16 ((UINT16)0xFFFF)
#define MAX_UINT8 ((UINT8)0xFF)
#define ARRAY_SIZE(Array) (sizeof (Array) / sizeof ((Array)[0]))
Does your Reviewed-by cover that as well?
> >-----Original Message-----
> >From: Laszlo Ersek [mailto:lersek@redhat.com]
> >Sent: Monday, December 03, 2018 9:06 PM
> >To: Ard Biesheuvel <ard.biesheuvel@linaro.org>; edk2-devel@lists.01.org
> >Cc: Zhu, Yonghong <yonghong.zhu@intel.com>; Gao, Liming
> ><liming.gao@intel.com>; Feng, Bob C <bob.c.feng@intel.com>; Carsey, Jaben
> ><jaben.carsey@intel.com>
> >Subject: Re: [PATCH v2 4/6] BaseTools/DevicePath: use MAX_UINT16 as
> >default device path max size
> >
> >On 11/30/18 23:45, Ard Biesheuvel wrote:
> >> Replace the default size limit of IsDevicePathValid() with a value
> >> that does not depend on the native word size of the build host.
> >>
> >> 64 KB seems sufficient as the upper bound of a device path handled
> >> by UEFI.
> >>
> >> Contributed-under: TianoCore Contribution Agreement 1.1
> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> >> Reviewed-by: Jaben Carsey <jaben.carsey@intel.com>
> >> ---
> >> BaseTools/Source/C/DevicePath/DevicePathUtilities.c | 4 ++--
> >> 1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/BaseTools/Source/C/DevicePath/DevicePathUtilities.c
> >b/BaseTools/Source/C/DevicePath/DevicePathUtilities.c
> >> index d4ec2742b7c8..ba7f83e53070 100644
> >> --- a/BaseTools/Source/C/DevicePath/DevicePathUtilities.c
> >> +++ b/BaseTools/Source/C/DevicePath/DevicePathUtilities.c
> >> @@ -62,7 +62,7 @@ IsDevicePathValid (
> >> ASSERT (DevicePath != NULL);
> >>
> >> if (MaxSize == 0) {
> >> - MaxSize = MAX_UINTN;
> >> + MaxSize = MAX_UINT16;
> >> }
> >>
> >> //
> >> @@ -78,7 +78,7 @@ IsDevicePathValid (
> >> return FALSE;
> >> }
> >>
> >> - if (NodeLength > MAX_UINTN - Size) {
> >> + if (NodeLength > MAX_UINT16 - Size) {
> >> return FALSE;
> >> }
> >> Size += NodeLength;
> >>
> >
> >I'm somewhat undecided about this patch.
> >
> >(1) IsDevicePathValid() also exists in:
> >
> >- MdePkg/Library/UefiDevicePathLib/DevicePathUtilities.c
> >- MdePkg/Library/UefiDevicePathLibDevicePathProtocol/UefiDevicePathLib.c
> >
> >Both have:
> >
> > if (MaxSize == 0) {
> > MaxSize = MAX_UINTN;
> > }
> >
> >Relative to those, this change departs quite strongly.
> >
> >
> >(2) In addition, a single device path node may extend up to 64KB. That
> >would be pathologic, yes, but the option is there.
> >
> >
> >... Of course, we are discussing theoretical limits. Still I'd feel more
> >comfortable with MAX_UINT32. Lifting the limit from 64K to 4G wouldn't
> >cost us anything (in development effort), it would be a no-op on 32-bit
> >build hosts, it would be a theoretical-only change on 64-bit build
> >hosts, and it would leave us with a larger "safety margin".
> >
> >I won't insist, but I thought I should raise this. (Sorry if this has
> >been discussed under v1 already.) If you agree, no need to repost (from
> >my side anyway) just for this.
> >
> >With or without the update:
> >
> >Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> >
> >Thanks
> >Laszlo
^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH v2 4/6] BaseTools/DevicePath: use MAX_UINT16 as default device path max size
2018-12-05 7:42 ` Ard Biesheuvel
@ 2018-12-05 7:53 ` Gao, Liming
0 siblings, 0 replies; 28+ messages in thread
From: Gao, Liming @ 2018-12-05 7:53 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Laszlo Ersek, edk2-devel@lists.01.org, Zhu, Yonghong, Feng, Bob C,
Carsey, Jaben
Yes. The change is necessary.
> -----Original Message-----
> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> Sent: Wednesday, December 5, 2018 3:42 PM
> To: Gao, Liming <liming.gao@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>; edk2-devel@lists.01.org; Zhu, Yonghong <yonghong.zhu@intel.com>; Feng, Bob C
> <bob.c.feng@intel.com>; Carsey, Jaben <jaben.carsey@intel.com>
> Subject: Re: [PATCH v2 4/6] BaseTools/DevicePath: use MAX_UINT16 as default device path max size
>
> On Wed, 5 Dec 2018 at 01:04, Gao, Liming <liming.gao@intel.com> wrote:
> >
> > Laszlo:
> > I agree with you. MAX_UINT32 is more comfortable.
> >
>
> Liming,
>
> No definitions for MAX_UINT32 exist currently in BaseTools, so I will
> have to add the following:
>
> diff --git a/BaseTools/Source/C/Common/CommonLib.h
> b/BaseTools/Source/C/Common/CommonLib.h
> index b1c6c00a3478..1c40180329c4 100644
> --- a/BaseTools/Source/C/Common/CommonLib.h
> +++ b/BaseTools/Source/C/Common/CommonLib.h
> @@ -23,6 +23,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND,
> EITHER EXPRESS OR IMPLIED.
> #define MAX_LONG_FILE_PATH 500
>
> #define MAX_UINT64 ((UINT64)0xFFFFFFFFFFFFFFFFULL)
> +#define MAX_UINT32 ((UINT32)0xFFFFFFFFULL)
> #define MAX_UINT16 ((UINT16)0xFFFF)
> #define MAX_UINT8 ((UINT8)0xFF)
> #define ARRAY_SIZE(Array) (sizeof (Array) / sizeof ((Array)[0]))
>
> Does your Reviewed-by cover that as well?
>
>
>
>
> > >-----Original Message-----
> > >From: Laszlo Ersek [mailto:lersek@redhat.com]
> > >Sent: Monday, December 03, 2018 9:06 PM
> > >To: Ard Biesheuvel <ard.biesheuvel@linaro.org>; edk2-devel@lists.01.org
> > >Cc: Zhu, Yonghong <yonghong.zhu@intel.com>; Gao, Liming
> > ><liming.gao@intel.com>; Feng, Bob C <bob.c.feng@intel.com>; Carsey, Jaben
> > ><jaben.carsey@intel.com>
> > >Subject: Re: [PATCH v2 4/6] BaseTools/DevicePath: use MAX_UINT16 as
> > >default device path max size
> > >
> > >On 11/30/18 23:45, Ard Biesheuvel wrote:
> > >> Replace the default size limit of IsDevicePathValid() with a value
> > >> that does not depend on the native word size of the build host.
> > >>
> > >> 64 KB seems sufficient as the upper bound of a device path handled
> > >> by UEFI.
> > >>
> > >> Contributed-under: TianoCore Contribution Agreement 1.1
> > >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > >> Reviewed-by: Jaben Carsey <jaben.carsey@intel.com>
> > >> ---
> > >> BaseTools/Source/C/DevicePath/DevicePathUtilities.c | 4 ++--
> > >> 1 file changed, 2 insertions(+), 2 deletions(-)
> > >>
> > >> diff --git a/BaseTools/Source/C/DevicePath/DevicePathUtilities.c
> > >b/BaseTools/Source/C/DevicePath/DevicePathUtilities.c
> > >> index d4ec2742b7c8..ba7f83e53070 100644
> > >> --- a/BaseTools/Source/C/DevicePath/DevicePathUtilities.c
> > >> +++ b/BaseTools/Source/C/DevicePath/DevicePathUtilities.c
> > >> @@ -62,7 +62,7 @@ IsDevicePathValid (
> > >> ASSERT (DevicePath != NULL);
> > >>
> > >> if (MaxSize == 0) {
> > >> - MaxSize = MAX_UINTN;
> > >> + MaxSize = MAX_UINT16;
> > >> }
> > >>
> > >> //
> > >> @@ -78,7 +78,7 @@ IsDevicePathValid (
> > >> return FALSE;
> > >> }
> > >>
> > >> - if (NodeLength > MAX_UINTN - Size) {
> > >> + if (NodeLength > MAX_UINT16 - Size) {
> > >> return FALSE;
> > >> }
> > >> Size += NodeLength;
> > >>
> > >
> > >I'm somewhat undecided about this patch.
> > >
> > >(1) IsDevicePathValid() also exists in:
> > >
> > >- MdePkg/Library/UefiDevicePathLib/DevicePathUtilities.c
> > >- MdePkg/Library/UefiDevicePathLibDevicePathProtocol/UefiDevicePathLib.c
> > >
> > >Both have:
> > >
> > > if (MaxSize == 0) {
> > > MaxSize = MAX_UINTN;
> > > }
> > >
> > >Relative to those, this change departs quite strongly.
> > >
> > >
> > >(2) In addition, a single device path node may extend up to 64KB. That
> > >would be pathologic, yes, but the option is there.
> > >
> > >
> > >... Of course, we are discussing theoretical limits. Still I'd feel more
> > >comfortable with MAX_UINT32. Lifting the limit from 64K to 4G wouldn't
> > >cost us anything (in development effort), it would be a no-op on 32-bit
> > >build hosts, it would be a theoretical-only change on 64-bit build
> > >hosts, and it would leave us with a larger "safety margin".
> > >
> > >I won't insist, but I thought I should raise this. (Sorry if this has
> > >been discussed under v1 already.) If you agree, no need to repost (from
> > >my side anyway) just for this.
> > >
> > >With or without the update:
> > >
> > >Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> > >
> > >Thanks
> > >Laszlo
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 0/6] BaseTools: get rid of MAX_UINTN
2018-12-05 0:04 ` [PATCH v2 0/6] BaseTools: get rid " Gao, Liming
@ 2018-12-05 8:12 ` Ard Biesheuvel
0 siblings, 0 replies; 28+ messages in thread
From: Ard Biesheuvel @ 2018-12-05 8:12 UTC (permalink / raw)
To: Gao, Liming
Cc: edk2-devel@lists.01.org, Carsey, Jaben, Laszlo Ersek,
Philippe Mathieu-Daudé
On Wed, 5 Dec 2018 at 01:04, Gao, Liming <liming.gao@intel.com> wrote:
>
> Reviewed-by: Liming Gao <liming.gao@intel.com> for this serials.
>
Thanks all
Series pushed as 64ab2c82e8f6..8efc6d84ca41
(with the requested MAX_UINT16 -> MAX_UINT32 change applied)
> On patch 4, I have the same comments to Laszlo.
>
> >-----Original Message-----
> >From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ard
> >Biesheuvel
> >Sent: Saturday, December 01, 2018 6:46 AM
> >To: edk2-devel@lists.01.org
> >Cc: Gao, Liming <liming.gao@intel.com>; Carsey, Jaben
> ><jaben.carsey@intel.com>; Laszlo Ersek <lersek@redhat.com>
> >Subject: [edk2] [PATCH v2 0/6] BaseTools: get rid of MAX_UINTN
> >
> >There should be no reason for the build tools to care about the native
> >word size of a particular target, so relying on a definition of MAX_UINTN
> >is definitely wrong, and most likely inaccurate on 32-bit build hosts.
> >
> >So refactor the code in CommonLib and DevicePath so we no longer rely
> >on this definition.
> >
> >Changes since v1:
> >- miss type change in #1 causing a build failure on MSVC
> >- add acks from Jaben
> >
> >Cc: Laszlo Ersek <lersek@redhat.com>
> >Cc: Yonghong Zhu <yonghong.zhu@intel.com>
> >Cc: Liming Gao <liming.gao@intel.com>
> >Cc: Bob Feng <bob.c.feng@intel.com>
> >Cc: Jaben Carsey <jaben.carsey@intel.com>
> >
> >Ard Biesheuvel (6):
> > BaseTools/CommonLib: avoid using 'native' word size in IP address
> > handling
> > BaseTools/CommonLib: use explicit 64-bit type in Strtoi()
> > BaseTools/DevicePath: use explicit 64-bit number parsing routines
> > BaseTools/DevicePath: use MAX_UINT16 as default device path max size
> > BaseTools/CommonLib: get rid of 'native' type string parsing routines
> > BaseTools/CommonLib: drop definition of MAX_UINTN
> >
> > BaseTools/Source/C/Common/CommonLib.h | 25 ---
> > BaseTools/Source/C/Common/CommonLib.c | 206 ++----------------
> > .../Source/C/DevicePath/DevicePathFromText.c | 4 +-
> > .../Source/C/DevicePath/DevicePathUtilities.c | 4 +-
> > 4 files changed, 25 insertions(+), 214 deletions(-)
> >
> >--
> >2.19.1
> >
> >_______________________________________________
> >edk2-devel mailing list
> >edk2-devel@lists.01.org
> >https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 6/6] BaseTools/CommonLib: drop definition of MAX_UINTN
2018-12-03 13:08 ` Laszlo Ersek
@ 2018-12-11 7:11 ` David F.
2018-12-11 15:45 ` Laszlo Ersek
0 siblings, 1 reply; 28+ messages in thread
From: David F. @ 2018-12-11 7:11 UTC (permalink / raw)
To: Laszlo Ersek
Cc: ard.biesheuvel, edk2 developers list, Carsey, Jaben, Gao, Liming
Not sure why you'd take that out when someone using UINTN for variables may
want to use MAX_UINTN ? Future may be different.
On Mon, Dec 3, 2018 at 5:08 AM Laszlo Ersek <lersek@redhat.com> wrote:
> On 11/30/18 23:45, Ard Biesheuvel wrote:
> > The maximum value that can be represented by the native word size
> > of the *target* should be irrelevant when compiling tools that
> > run on the build *host*. So drop the definition of MAX_UINTN, now
> > that we no longer use it.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > Reviewed-by: Jaben Carsey <jaben.carsey@intel.com>
> > ---
> > BaseTools/Source/C/Common/CommonLib.h | 1 -
> > 1 file changed, 1 deletion(-)
> >
> > diff --git a/BaseTools/Source/C/Common/CommonLib.h
> b/BaseTools/Source/C/Common/CommonLib.h
> > index 6930d9227b87..b1c6c00a3478 100644
> > --- a/BaseTools/Source/C/Common/CommonLib.h
> > +++ b/BaseTools/Source/C/Common/CommonLib.h
> > @@ -22,7 +22,6 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND,
> EITHER EXPRESS OR IMPLIED.
> >
> > #define MAX_LONG_FILE_PATH 500
> >
> > -#define MAX_UINTN MAX_ADDRESS
> > #define MAX_UINT64 ((UINT64)0xFFFFFFFFFFFFFFFFULL)
> > #define MAX_UINT16 ((UINT16)0xFFFF)
> > #define MAX_UINT8 ((UINT8)0xFF)
> >
>
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 6/6] BaseTools/CommonLib: drop definition of MAX_UINTN
2018-12-11 7:11 ` David F.
@ 2018-12-11 15:45 ` Laszlo Ersek
2018-12-11 22:53 ` David F.
0 siblings, 1 reply; 28+ messages in thread
From: Laszlo Ersek @ 2018-12-11 15:45 UTC (permalink / raw)
To: David F.; +Cc: ard.biesheuvel, edk2 developers list, Carsey, Jaben, Gao, Liming
On 12/11/18 08:11, David F. wrote:
> Not sure why you'd take that out when someone using UINTN for variables may
> want to use MAX_UINTN ? Future may be different.
The UINTN type comes from the UEFI spec:
Unsigned value of native width. (4 bytes on supported 32-bit
processor instructions, 8 bytes on supported 64-bit processor
instructions, 16 bytes on supported 128-bit processor instructions)
In this sense, "native" refers to the firmware execution environment.
The firmware execution environment need not have anything in common with
the build environment. (You can build 32-bit ARM firmware on X64 hosts.)
In such a scenario, using UINTN *at all* is fraught with
misunderstandings. It *would* be possible to use UINTN as it applies to
the build (= hosted) environment, and in that sense MAX_UINTN would also
be possible to define. However, the code being removed (= defining
MAX_UINTN as MAX_ADDRESS) proves that that approach would be very easy
to misunderstand and misuse. People could easily mistake it for applying
to the firmware execution environment.
UINT32 and UINT64 are not affected by this ambiguity.
Optimally, given that the build utilities target a hosted C runtime,
they should use standard C types, such as "unsigned int", or e.g.
"uint32_t". Together with standard C macros expressing limits, such as
UINT_MAX (from <limits.h>) and UINT32_MAX (from <stdint.h>).
Clearly no-one has capacity to clean up BaseTools like this. For
starters, we should at least remove whatever actively causes confusion.
Thanks,
Laszlo
> On Mon, Dec 3, 2018 at 5:08 AM Laszlo Ersek <lersek@redhat.com> wrote:
>
>> On 11/30/18 23:45, Ard Biesheuvel wrote:
>>> The maximum value that can be represented by the native word size
>>> of the *target* should be irrelevant when compiling tools that
>>> run on the build *host*. So drop the definition of MAX_UINTN, now
>>> that we no longer use it.
>>>
>>> Contributed-under: TianoCore Contribution Agreement 1.1
>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>> Reviewed-by: Jaben Carsey <jaben.carsey@intel.com>
>>> ---
>>> BaseTools/Source/C/Common/CommonLib.h | 1 -
>>> 1 file changed, 1 deletion(-)
>>>
>>> diff --git a/BaseTools/Source/C/Common/CommonLib.h
>> b/BaseTools/Source/C/Common/CommonLib.h
>>> index 6930d9227b87..b1c6c00a3478 100644
>>> --- a/BaseTools/Source/C/Common/CommonLib.h
>>> +++ b/BaseTools/Source/C/Common/CommonLib.h
>>> @@ -22,7 +22,6 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND,
>> EITHER EXPRESS OR IMPLIED.
>>>
>>> #define MAX_LONG_FILE_PATH 500
>>>
>>> -#define MAX_UINTN MAX_ADDRESS
>>> #define MAX_UINT64 ((UINT64)0xFFFFFFFFFFFFFFFFULL)
>>> #define MAX_UINT16 ((UINT16)0xFFFF)
>>> #define MAX_UINT8 ((UINT8)0xFF)
>>>
>>
>> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
>> _______________________________________________
>> edk2-devel mailing list
>> edk2-devel@lists.01.org
>> https://lists.01.org/mailman/listinfo/edk2-devel
>>
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 6/6] BaseTools/CommonLib: drop definition of MAX_UINTN
2018-12-11 15:45 ` Laszlo Ersek
@ 2018-12-11 22:53 ` David F.
2018-12-11 22:55 ` Ard Biesheuvel
0 siblings, 1 reply; 28+ messages in thread
From: David F. @ 2018-12-11 22:53 UTC (permalink / raw)
To: Laszlo Ersek
Cc: Ard Biesheuvel, edk2 developers list, Carsey, Jaben, Gao, Liming
I don't know, to me it's very clear that UINTN is talking about the target,
just like size_t would be.
There are/were a bunch of API's using UINTN so using UINTN was desirable,
and where needed UINTN_MAX.
I just don't see an advantage to removing it. Do see disadvantage to
removing it for breaking existing code and for those that want the "native"
(best/fasted/most efficient) int size for the processor (similar again to
size_t)
On Tue, Dec 11, 2018 at 7:46 AM Laszlo Ersek <lersek@redhat.com> wrote:
> On 12/11/18 08:11, David F. wrote:
> > Not sure why you'd take that out when someone using UINTN for variables
> may
> > want to use MAX_UINTN ? Future may be different.
>
> The UINTN type comes from the UEFI spec:
>
> Unsigned value of native width. (4 bytes on supported 32-bit
> processor instructions, 8 bytes on supported 64-bit processor
> instructions, 16 bytes on supported 128-bit processor instructions)
>
> In this sense, "native" refers to the firmware execution environment.
> The firmware execution environment need not have anything in common with
> the build environment. (You can build 32-bit ARM firmware on X64 hosts.)
> In such a scenario, using UINTN *at all* is fraught with
> misunderstandings. It *would* be possible to use UINTN as it applies to
> the build (= hosted) environment, and in that sense MAX_UINTN would also
> be possible to define. However, the code being removed (= defining
> MAX_UINTN as MAX_ADDRESS) proves that that approach would be very easy
> to misunderstand and misuse. People could easily mistake it for applying
> to the firmware execution environment.
>
> UINT32 and UINT64 are not affected by this ambiguity.
>
> Optimally, given that the build utilities target a hosted C runtime,
> they should use standard C types, such as "unsigned int", or e.g.
> "uint32_t". Together with standard C macros expressing limits, such as
> UINT_MAX (from <limits.h>) and UINT32_MAX (from <stdint.h>).
>
> Clearly no-one has capacity to clean up BaseTools like this. For
> starters, we should at least remove whatever actively causes confusion.
>
> Thanks,
> Laszlo
>
> > On Mon, Dec 3, 2018 at 5:08 AM Laszlo Ersek <lersek@redhat.com> wrote:
> >
> >> On 11/30/18 23:45, Ard Biesheuvel wrote:
> >>> The maximum value that can be represented by the native word size
> >>> of the *target* should be irrelevant when compiling tools that
> >>> run on the build *host*. So drop the definition of MAX_UINTN, now
> >>> that we no longer use it.
> >>>
> >>> Contributed-under: TianoCore Contribution Agreement 1.1
> >>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> >>> Reviewed-by: Jaben Carsey <jaben.carsey@intel.com>
> >>> ---
> >>> BaseTools/Source/C/Common/CommonLib.h | 1 -
> >>> 1 file changed, 1 deletion(-)
> >>>
> >>> diff --git a/BaseTools/Source/C/Common/CommonLib.h
> >> b/BaseTools/Source/C/Common/CommonLib.h
> >>> index 6930d9227b87..b1c6c00a3478 100644
> >>> --- a/BaseTools/Source/C/Common/CommonLib.h
> >>> +++ b/BaseTools/Source/C/Common/CommonLib.h
> >>> @@ -22,7 +22,6 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND,
> >> EITHER EXPRESS OR IMPLIED.
> >>>
> >>> #define MAX_LONG_FILE_PATH 500
> >>>
> >>> -#define MAX_UINTN MAX_ADDRESS
> >>> #define MAX_UINT64 ((UINT64)0xFFFFFFFFFFFFFFFFULL)
> >>> #define MAX_UINT16 ((UINT16)0xFFFF)
> >>> #define MAX_UINT8 ((UINT8)0xFF)
> >>>
> >>
> >> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> >> _______________________________________________
> >> edk2-devel mailing list
> >> edk2-devel@lists.01.org
> >> https://lists.01.org/mailman/listinfo/edk2-devel
> >>
> >
>
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 6/6] BaseTools/CommonLib: drop definition of MAX_UINTN
2018-12-11 22:53 ` David F.
@ 2018-12-11 22:55 ` Ard Biesheuvel
2018-12-11 23:03 ` David F.
0 siblings, 1 reply; 28+ messages in thread
From: Ard Biesheuvel @ 2018-12-11 22:55 UTC (permalink / raw)
To: David F.
Cc: Laszlo Ersek, edk2-devel@lists.01.org, Carsey, Jaben, Gao, Liming
On Tue, 11 Dec 2018 at 23:53, David F. <df7729@gmail.com> wrote:
>
> I don't know, to me it's very clear that UINTN is talking about the target, just like size_t would be.
>
But which target? This change is against the source of the BaseTools,
which are host tools that can be used to build a single target image
consisting of 32-bit and 64-bit modules. So which size is the native
size in this case?
> There are/were a bunch of API's using UINTN so using UINTN was desirable, and where needed UINTN_MAX.
>
> I just don't see an advantage to removing it. Do see disadvantage to removing it for breaking existing code and for those that want the "native" (best/fasted/most efficient) int size for the processor (similar again to size_t)
>
> On Tue, Dec 11, 2018 at 7:46 AM Laszlo Ersek <lersek@redhat.com> wrote:
>>
>> On 12/11/18 08:11, David F. wrote:
>> > Not sure why you'd take that out when someone using UINTN for variables may
>> > want to use MAX_UINTN ? Future may be different.
>>
>> The UINTN type comes from the UEFI spec:
>>
>> Unsigned value of native width. (4 bytes on supported 32-bit
>> processor instructions, 8 bytes on supported 64-bit processor
>> instructions, 16 bytes on supported 128-bit processor instructions)
>>
>> In this sense, "native" refers to the firmware execution environment.
>> The firmware execution environment need not have anything in common with
>> the build environment. (You can build 32-bit ARM firmware on X64 hosts.)
>> In such a scenario, using UINTN *at all* is fraught with
>> misunderstandings. It *would* be possible to use UINTN as it applies to
>> the build (= hosted) environment, and in that sense MAX_UINTN would also
>> be possible to define. However, the code being removed (= defining
>> MAX_UINTN as MAX_ADDRESS) proves that that approach would be very easy
>> to misunderstand and misuse. People could easily mistake it for applying
>> to the firmware execution environment.
>>
>> UINT32 and UINT64 are not affected by this ambiguity.
>>
>> Optimally, given that the build utilities target a hosted C runtime,
>> they should use standard C types, such as "unsigned int", or e.g.
>> "uint32_t". Together with standard C macros expressing limits, such as
>> UINT_MAX (from <limits.h>) and UINT32_MAX (from <stdint.h>).
>>
>> Clearly no-one has capacity to clean up BaseTools like this. For
>> starters, we should at least remove whatever actively causes confusion.
>>
>> Thanks,
>> Laszlo
>>
>> > On Mon, Dec 3, 2018 at 5:08 AM Laszlo Ersek <lersek@redhat.com> wrote:
>> >
>> >> On 11/30/18 23:45, Ard Biesheuvel wrote:
>> >>> The maximum value that can be represented by the native word size
>> >>> of the *target* should be irrelevant when compiling tools that
>> >>> run on the build *host*. So drop the definition of MAX_UINTN, now
>> >>> that we no longer use it.
>> >>>
>> >>> Contributed-under: TianoCore Contribution Agreement 1.1
>> >>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> >>> Reviewed-by: Jaben Carsey <jaben.carsey@intel.com>
>> >>> ---
>> >>> BaseTools/Source/C/Common/CommonLib.h | 1 -
>> >>> 1 file changed, 1 deletion(-)
>> >>>
>> >>> diff --git a/BaseTools/Source/C/Common/CommonLib.h
>> >> b/BaseTools/Source/C/Common/CommonLib.h
>> >>> index 6930d9227b87..b1c6c00a3478 100644
>> >>> --- a/BaseTools/Source/C/Common/CommonLib.h
>> >>> +++ b/BaseTools/Source/C/Common/CommonLib.h
>> >>> @@ -22,7 +22,6 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND,
>> >> EITHER EXPRESS OR IMPLIED.
>> >>>
>> >>> #define MAX_LONG_FILE_PATH 500
>> >>>
>> >>> -#define MAX_UINTN MAX_ADDRESS
>> >>> #define MAX_UINT64 ((UINT64)0xFFFFFFFFFFFFFFFFULL)
>> >>> #define MAX_UINT16 ((UINT16)0xFFFF)
>> >>> #define MAX_UINT8 ((UINT8)0xFF)
>> >>>
>> >>
>> >> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
>> >> _______________________________________________
>> >> edk2-devel mailing list
>> >> edk2-devel@lists.01.org
>> >> https://lists.01.org/mailman/listinfo/edk2-devel
>> >>
>> >
>>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 6/6] BaseTools/CommonLib: drop definition of MAX_UINTN
2018-12-11 22:55 ` Ard Biesheuvel
@ 2018-12-11 23:03 ` David F.
0 siblings, 0 replies; 28+ messages in thread
From: David F. @ 2018-12-11 23:03 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Laszlo Ersek, edk2 developers list, Carsey, Jaben, Gao, Liming
I missed that it was for the build-tool source itself and not for the
targets that are built using edk2 and the API itself.
On Tue, Dec 11, 2018 at 2:55 PM Ard Biesheuvel <ard.biesheuvel@linaro.org>
wrote:
> On Tue, 11 Dec 2018 at 23:53, David F. <df7729@gmail.com> wrote:
> >
> > I don't know, to me it's very clear that UINTN is talking about the
> target, just like size_t would be.
> >
>
> But which target? This change is against the source of the BaseTools,
> which are host tools that can be used to build a single target image
> consisting of 32-bit and 64-bit modules. So which size is the native
> size in this case?
>
> > There are/were a bunch of API's using UINTN so using UINTN was
> desirable, and where needed UINTN_MAX.
> >
> > I just don't see an advantage to removing it. Do see disadvantage to
> removing it for breaking existing code and for those that want the "native"
> (best/fasted/most efficient) int size for the processor (similar again to
> size_t)
> >
> > On Tue, Dec 11, 2018 at 7:46 AM Laszlo Ersek <lersek@redhat.com> wrote:
> >>
> >> On 12/11/18 08:11, David F. wrote:
> >> > Not sure why you'd take that out when someone using UINTN for
> variables may
> >> > want to use MAX_UINTN ? Future may be different.
> >>
> >> The UINTN type comes from the UEFI spec:
> >>
> >> Unsigned value of native width. (4 bytes on supported 32-bit
> >> processor instructions, 8 bytes on supported 64-bit processor
> >> instructions, 16 bytes on supported 128-bit processor instructions)
> >>
> >> In this sense, "native" refers to the firmware execution environment.
> >> The firmware execution environment need not have anything in common with
> >> the build environment. (You can build 32-bit ARM firmware on X64 hosts.)
> >> In such a scenario, using UINTN *at all* is fraught with
> >> misunderstandings. It *would* be possible to use UINTN as it applies to
> >> the build (= hosted) environment, and in that sense MAX_UINTN would also
> >> be possible to define. However, the code being removed (= defining
> >> MAX_UINTN as MAX_ADDRESS) proves that that approach would be very easy
> >> to misunderstand and misuse. People could easily mistake it for applying
> >> to the firmware execution environment.
> >>
> >> UINT32 and UINT64 are not affected by this ambiguity.
> >>
> >> Optimally, given that the build utilities target a hosted C runtime,
> >> they should use standard C types, such as "unsigned int", or e.g.
> >> "uint32_t". Together with standard C macros expressing limits, such as
> >> UINT_MAX (from <limits.h>) and UINT32_MAX (from <stdint.h>).
> >>
> >> Clearly no-one has capacity to clean up BaseTools like this. For
> >> starters, we should at least remove whatever actively causes confusion.
> >>
> >> Thanks,
> >> Laszlo
> >>
> >> > On Mon, Dec 3, 2018 at 5:08 AM Laszlo Ersek <lersek@redhat.com>
> wrote:
> >> >
> >> >> On 11/30/18 23:45, Ard Biesheuvel wrote:
> >> >>> The maximum value that can be represented by the native word size
> >> >>> of the *target* should be irrelevant when compiling tools that
> >> >>> run on the build *host*. So drop the definition of MAX_UINTN, now
> >> >>> that we no longer use it.
> >> >>>
> >> >>> Contributed-under: TianoCore Contribution Agreement 1.1
> >> >>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> >> >>> Reviewed-by: Jaben Carsey <jaben.carsey@intel.com>
> >> >>> ---
> >> >>> BaseTools/Source/C/Common/CommonLib.h | 1 -
> >> >>> 1 file changed, 1 deletion(-)
> >> >>>
> >> >>> diff --git a/BaseTools/Source/C/Common/CommonLib.h
> >> >> b/BaseTools/Source/C/Common/CommonLib.h
> >> >>> index 6930d9227b87..b1c6c00a3478 100644
> >> >>> --- a/BaseTools/Source/C/Common/CommonLib.h
> >> >>> +++ b/BaseTools/Source/C/Common/CommonLib.h
> >> >>> @@ -22,7 +22,6 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND,
> >> >> EITHER EXPRESS OR IMPLIED.
> >> >>>
> >> >>> #define MAX_LONG_FILE_PATH 500
> >> >>>
> >> >>> -#define MAX_UINTN MAX_ADDRESS
> >> >>> #define MAX_UINT64 ((UINT64)0xFFFFFFFFFFFFFFFFULL)
> >> >>> #define MAX_UINT16 ((UINT16)0xFFFF)
> >> >>> #define MAX_UINT8 ((UINT8)0xFF)
> >> >>>
> >> >>
> >> >> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> >> >> _______________________________________________
> >> >> edk2-devel mailing list
> >> >> edk2-devel@lists.01.org
> >> >> https://lists.01.org/mailman/listinfo/edk2-devel
> >> >>
> >> >
> >>
>
^ permalink raw reply [flat|nested] 28+ messages in thread
end of thread, other threads:[~2018-12-11 23:03 UTC | newest]
Thread overview: 28+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-11-30 22:45 [PATCH v2 0/6] BaseTools: get rid of MAX_UINTN Ard Biesheuvel
2018-11-30 22:45 ` [PATCH v2 1/6] BaseTools/CommonLib: avoid using 'native' word size in IP address handling Ard Biesheuvel
2018-12-03 10:25 ` Philippe Mathieu-Daudé
2018-12-03 12:50 ` Laszlo Ersek
2018-11-30 22:45 ` [PATCH v2 2/6] BaseTools/CommonLib: use explicit 64-bit type in Strtoi() Ard Biesheuvel
2018-12-03 10:25 ` Philippe Mathieu-Daudé
2018-12-03 12:52 ` Laszlo Ersek
2018-11-30 22:45 ` [PATCH v2 3/6] BaseTools/DevicePath: use explicit 64-bit number parsing routines Ard Biesheuvel
2018-12-03 10:25 ` Philippe Mathieu-Daudé
2018-12-03 12:55 ` Laszlo Ersek
2018-11-30 22:45 ` [PATCH v2 4/6] BaseTools/DevicePath: use MAX_UINT16 as default device path max size Ard Biesheuvel
2018-12-03 13:05 ` Laszlo Ersek
2018-12-05 0:04 ` Gao, Liming
2018-12-05 7:42 ` Ard Biesheuvel
2018-12-05 7:53 ` Gao, Liming
2018-11-30 22:45 ` [PATCH v2 5/6] BaseTools/CommonLib: get rid of 'native' type string parsing routines Ard Biesheuvel
2018-12-03 10:27 ` Philippe Mathieu-Daudé
2018-12-03 13:08 ` Laszlo Ersek
2018-11-30 22:45 ` [PATCH v2 6/6] BaseTools/CommonLib: drop definition of MAX_UINTN Ard Biesheuvel
2018-12-03 10:28 ` Philippe Mathieu-Daudé
2018-12-03 13:08 ` Laszlo Ersek
2018-12-11 7:11 ` David F.
2018-12-11 15:45 ` Laszlo Ersek
2018-12-11 22:53 ` David F.
2018-12-11 22:55 ` Ard Biesheuvel
2018-12-11 23:03 ` David F.
2018-12-05 0:04 ` [PATCH v2 0/6] BaseTools: get rid " Gao, Liming
2018-12-05 8:12 ` Ard Biesheuvel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox