[PATCH 0/6] BaseTools: get rid of MAX_UINTN

[PATCH 0/6] BaseTools: get rid of MAX_UINTN

[Ard Biesheuvel]

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.

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>

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

[PATCH 1/6] BaseTools/CommonLib: avoid using 'native' word size in IP address handling

[Ard Biesheuvel]

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..bea6af0a45b1 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;
+  UINTN                  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

[PATCH 2/6] BaseTools/CommonLib: use explicit 64-bit type in Strtoi()

[Ard Biesheuvel]

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>
---
 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 bea6af0a45b1..c5e32b1292e0 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

[PATCH 3/6] BaseTools/DevicePath: use explicit 64-bit number parsing routines

[Ard Biesheuvel]

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>
---
 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

[PATCH 4/6] BaseTools/DevicePath: use MAX_UINT16 as default device path max size

[Ard Biesheuvel]

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>
---
 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

[PATCH 5/6] BaseTools/CommonLib: get rid of 'native' type string parsing routines

[Ard Biesheuvel]

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 c5e32b1292e0..9142a9a7eda3 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

[PATCH 6/6] BaseTools/CommonLib: drop definition of MAX_UINTN

[Ard Biesheuvel]

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>
---
 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

Re: [PATCH 1/6] BaseTools/CommonLib: avoid using 'native' word size in IP address handling

[Gao, Liming]

Ard:
  I mean the build error. Besides, what test have you done with this patch set?

CommonLib.c(1651): error C2220: warning treated as error - no 'object' file generated
CommonLib.c(1651): warning C4133: 'function': incompatible types - from 'UINTN *' to 'UINT64 *'
NMAKE : fatal error U1077: '"C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\BIN\cl.exe"' : return code '0x2'

Below > +  UINTN                  Uint64; ==> > +  UINT64                  Uint64;
> -----Original Message-----
> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> Sent: Thursday, November 29, 2018 8:31 PM
> To: edk2-devel@lists.01.org
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Laszlo Ersek <lersek@redhat.com>; Zhu, Yonghong <yonghong.zhu@intel.com>; Gao,
> Liming <liming.gao@intel.com>; Feng, Bob C <bob.c.feng@intel.com>
> Subject: [PATCH 1/6] BaseTools/CommonLib: avoid using 'native' word size in IP address handling
> 
> 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..bea6af0a45b1 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;
> +  UINTN                  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

Re: [PATCH 1/6] BaseTools/CommonLib: avoid using 'native' word size in IP address handling

[Ard Biesheuvel]

On Thu, 29 Nov 2018 at 16:11, Gao, Liming <liming.gao@intel.com> wrote:
>
> Ard:
>   I mean the build error. Besides, what test have you done with this patch set?
>
> CommonLib.c(1651): error C2220: warning treated as error - no 'object' file generated
> CommonLib.c(1651): warning C4133: 'function': incompatible types - from 'UINTN *' to 'UINT64 *'
> NMAKE : fatal error U1077: '"C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\BIN\cl.exe"' : return code '0x2'
>

Apologies, i missed this change at line 1624

-  UINTN                  Uint64;
+  UINT64                 Uint64;

It builds fine with GCC though.

> Below > +  UINTN                  Uint64; ==> > +  UINT64                  Uint64;
> > -----Original Message-----
> > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> > Sent: Thursday, November 29, 2018 8:31 PM
> > To: edk2-devel@lists.01.org
> > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Laszlo Ersek <lersek@redhat.com>; Zhu, Yonghong <yonghong.zhu@intel.com>; Gao,
> > Liming <liming.gao@intel.com>; Feng, Bob C <bob.c.feng@intel.com>
> > Subject: [PATCH 1/6] BaseTools/CommonLib: avoid using 'native' word size in IP address handling
> >
> > 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..bea6af0a45b1 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;
> > +  UINTN                  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
>

Re: [PATCH 1/6] BaseTools/CommonLib: avoid using 'native' word size in IP address handling

[Gao, Liming]

Do you verify which GCC arch? 32bit or 64bit or ARM?

> -----Original Message-----
> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> Sent: Thursday, November 29, 2018 11:14 PM
> To: Gao, Liming <liming.gao@intel.com>
> Cc: edk2-devel@lists.01.org; Laszlo Ersek <lersek@redhat.com>; Zhu, Yonghong <yonghong.zhu@intel.com>; Feng, Bob C
> <bob.c.feng@intel.com>
> Subject: Re: [PATCH 1/6] BaseTools/CommonLib: avoid using 'native' word size in IP address handling
> 
> On Thu, 29 Nov 2018 at 16:11, Gao, Liming <liming.gao@intel.com> wrote:
> >
> > Ard:
> >   I mean the build error. Besides, what test have you done with this patch set?
> >
> > CommonLib.c(1651): error C2220: warning treated as error - no 'object' file generated
> > CommonLib.c(1651): warning C4133: 'function': incompatible types - from 'UINTN *' to 'UINT64 *'
> > NMAKE : fatal error U1077: '"C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\BIN\cl.exe"' : return code '0x2'
> >
> 
> Apologies, i missed this change at line 1624
> 
> -  UINTN                  Uint64;
> +  UINT64                 Uint64;
> 
> It builds fine with GCC though.
> 
> > Below > +  UINTN                  Uint64; ==> > +  UINT64                  Uint64;
> > > -----Original Message-----
> > > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> > > Sent: Thursday, November 29, 2018 8:31 PM
> > > To: edk2-devel@lists.01.org
> > > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Laszlo Ersek <lersek@redhat.com>; Zhu, Yonghong <yonghong.zhu@intel.com>;
> Gao,
> > > Liming <liming.gao@intel.com>; Feng, Bob C <bob.c.feng@intel.com>
> > > Subject: [PATCH 1/6] BaseTools/CommonLib: avoid using 'native' word size in IP address handling
> > >
> > > 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..bea6af0a45b1 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;
> > > +  UINTN                  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
> >

Re: [PATCH 3/6] BaseTools/DevicePath: use explicit 64-bit number parsing routines

[Carsey, Jaben]

Reviewed-by: Jaben Carsey <jaben.carsey@intel.com>

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Ard Biesheuvel
> Sent: Thursday, November 29, 2018 4:31 AM
> To: edk2-devel@lists.01.org
> Cc: Laszlo Ersek <lersek@redhat.com>; Gao, Liming <liming.gao@intel.com>
> Subject: [edk2] [PATCH 3/6] BaseTools/DevicePath: use explicit 64-bit
> number parsing routines
> 
> 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>
> ---
>  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
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel

Re: [PATCH 2/6] BaseTools/CommonLib: use explicit 64-bit type in Strtoi()

[Carsey, Jaben]

Reviewed-by: Jaben Carsey <jaben.carsey@intel.com>

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Ard Biesheuvel
> Sent: Thursday, November 29, 2018 4:31 AM
> To: edk2-devel@lists.01.org
> Cc: Laszlo Ersek <lersek@redhat.com>; Gao, Liming <liming.gao@intel.com>
> Subject: [edk2] [PATCH 2/6] BaseTools/CommonLib: use explicit 64-bit type
> in Strtoi()
> 
> 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>
> ---
>  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 bea6af0a45b1..c5e32b1292e0 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
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel

Re: [PATCH 6/6] BaseTools/CommonLib: drop definition of MAX_UINTN

[Carsey, Jaben]

Reviewed-by: Jaben Carsey <jaben.carsey@intel.com>

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Ard Biesheuvel
> Sent: Thursday, November 29, 2018 4:31 AM
> To: edk2-devel@lists.01.org
> Cc: Laszlo Ersek <lersek@redhat.com>; Gao, Liming <liming.gao@intel.com>
> Subject: [edk2] [PATCH 6/6] BaseTools/CommonLib: drop definition of
> MAX_UINTN
> 
> 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>
> ---
>  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
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel

Re: [PATCH 1/6] BaseTools/CommonLib: avoid using 'native' word size in IP address handling

[Ard Biesheuvel]

On Thu, 29 Nov 2018 at 16:15, Gao, Liming <liming.gao@intel.com> wrote:
>
> Do you verify which GCC arch? 32bit or 64bit or ARM?
>

64-bit ARM

> > -----Original Message-----
> > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> > Sent: Thursday, November 29, 2018 11:14 PM
> > To: Gao, Liming <liming.gao@intel.com>
> > Cc: edk2-devel@lists.01.org; Laszlo Ersek <lersek@redhat.com>; Zhu, Yonghong <yonghong.zhu@intel.com>; Feng, Bob C
> > <bob.c.feng@intel.com>
> > Subject: Re: [PATCH 1/6] BaseTools/CommonLib: avoid using 'native' word size in IP address handling
> >
> > On Thu, 29 Nov 2018 at 16:11, Gao, Liming <liming.gao@intel.com> wrote:
> > >
> > > Ard:
> > >   I mean the build error. Besides, what test have you done with this patch set?
> > >
> > > CommonLib.c(1651): error C2220: warning treated as error - no 'object' file generated
> > > CommonLib.c(1651): warning C4133: 'function': incompatible types - from 'UINTN *' to 'UINT64 *'
> > > NMAKE : fatal error U1077: '"C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\BIN\cl.exe"' : return code '0x2'
> > >
> >
> > Apologies, i missed this change at line 1624
> >
> > -  UINTN                  Uint64;
> > +  UINT64                 Uint64;
> >
> > It builds fine with GCC though.
> >
> > > Below > +  UINTN                  Uint64; ==> > +  UINT64                  Uint64;
> > > > -----Original Message-----
> > > > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> > > > Sent: Thursday, November 29, 2018 8:31 PM
> > > > To: edk2-devel@lists.01.org
> > > > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Laszlo Ersek <lersek@redhat.com>; Zhu, Yonghong <yonghong.zhu@intel.com>;
> > Gao,
> > > > Liming <liming.gao@intel.com>; Feng, Bob C <bob.c.feng@intel.com>
> > > > Subject: [PATCH 1/6] BaseTools/CommonLib: avoid using 'native' word size in IP address handling
> > > >
> > > > 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..bea6af0a45b1 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;
> > > > +  UINTN                  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
> > >

Re: [PATCH 4/6] BaseTools/DevicePath: use MAX_UINT16 as default device path max size

[Carsey, Jaben]

Reviewed-by: Jaben Carsey <jaben.carsey@intel.com>

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Ard Biesheuvel
> Sent: Thursday, November 29, 2018 4:31 AM
> To: edk2-devel@lists.01.org
> Cc: Laszlo Ersek <lersek@redhat.com>; Gao, Liming <liming.gao@intel.com>
> Subject: [edk2] [PATCH 4/6] BaseTools/DevicePath: use MAX_UINT16 as
> default device path max size
> 
> 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>
> ---
>  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
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel

Re: [PATCH 1/6] BaseTools/CommonLib: avoid using 'native' word size in IP address handling

[Gao, Liming]

Ard: 
  OK. Will you send v2 patch to fix this issue?

> -----Original Message-----
> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> Sent: Thursday, November 29, 2018 11:18 PM
> To: Gao, Liming <liming.gao@intel.com>
> Cc: edk2-devel@lists.01.org; Laszlo Ersek <lersek@redhat.com>; Zhu, Yonghong <yonghong.zhu@intel.com>; Feng, Bob C
> <bob.c.feng@intel.com>
> Subject: Re: [PATCH 1/6] BaseTools/CommonLib: avoid using 'native' word size in IP address handling
> 
> On Thu, 29 Nov 2018 at 16:15, Gao, Liming <liming.gao@intel.com> wrote:
> >
> > Do you verify which GCC arch? 32bit or 64bit or ARM?
> >
> 
> 64-bit ARM
> 
> > > -----Original Message-----
> > > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> > > Sent: Thursday, November 29, 2018 11:14 PM
> > > To: Gao, Liming <liming.gao@intel.com>
> > > Cc: edk2-devel@lists.01.org; Laszlo Ersek <lersek@redhat.com>; Zhu, Yonghong <yonghong.zhu@intel.com>; Feng, Bob C
> > > <bob.c.feng@intel.com>
> > > Subject: Re: [PATCH 1/6] BaseTools/CommonLib: avoid using 'native' word size in IP address handling
> > >
> > > On Thu, 29 Nov 2018 at 16:11, Gao, Liming <liming.gao@intel.com> wrote:
> > > >
> > > > Ard:
> > > >   I mean the build error. Besides, what test have you done with this patch set?
> > > >
> > > > CommonLib.c(1651): error C2220: warning treated as error - no 'object' file generated
> > > > CommonLib.c(1651): warning C4133: 'function': incompatible types - from 'UINTN *' to 'UINT64 *'
> > > > NMAKE : fatal error U1077: '"C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\BIN\cl.exe"' : return code '0x2'
> > > >
> > >
> > > Apologies, i missed this change at line 1624
> > >
> > > -  UINTN                  Uint64;
> > > +  UINT64                 Uint64;
> > >
> > > It builds fine with GCC though.
> > >
> > > > Below > +  UINTN                  Uint64; ==> > +  UINT64                  Uint64;
> > > > > -----Original Message-----
> > > > > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> > > > > Sent: Thursday, November 29, 2018 8:31 PM
> > > > > To: edk2-devel@lists.01.org
> > > > > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Laszlo Ersek <lersek@redhat.com>; Zhu, Yonghong <yonghong.zhu@intel.com>;
> > > Gao,
> > > > > Liming <liming.gao@intel.com>; Feng, Bob C <bob.c.feng@intel.com>
> > > > > Subject: [PATCH 1/6] BaseTools/CommonLib: avoid using 'native' word size in IP address handling
> > > > >
> > > > > 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..bea6af0a45b1 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;
> > > > > +  UINTN                  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
> > > >