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

[Laszlo Ersek <lersek@redhat.com>]

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