Re: [edk2] [PATCH] MdeModulePkg/CoreDxe: Fixed compiler warning 'integer conversion resulted in a change of sign'

Subject: Re: [edk2] [PATCH] MdeModulePkg/CoreDxe: Fixed compiler warning 'integer conversion resulted in a change of sign'

From: Laszlo Ersek <lersek@redhat.com>

To: edk2-devel@lists.sourceforge.net

Date: 2012-07-10 20:52:40

On 07/09/12 21:34, Olivier Martin wrote:
> Dear MdeModulePkg,
>
> A few years back a change (svn rev10347 \u2013 07/04/2010) has been made in
> MdeModulePkg/Core/Dxe/Mem/Pool.c to fix GCC builds because enum were
> treated as UINT32. So the fix was:

> --- a/MdeModulePkg/Core/Dxe/Mem/Pool.c
> +++ b/MdeModulePkg/Core/Dxe/Mem/Pool.c
>
> @@ -128,7 +128,7 @@ LookupPoolHead (
>    // MemoryType values in the range 0x80000000..0xFFFFFFFF are reserved for use by UEFI
>    // OS loaders that are provided by operating system vendors
>    //
> -  if (MemoryType < 0) {
> +  if (MemoryType >= (INT32)0x80000000 && MemoryType <= (INT32)0xffffffff) {
>
>      for (Link = mPoolHeadList.ForwardLink; Link != &mPoolHeadList; Link = Link->ForwardLink) {
>        Pool = CR(Link, POOL, Link, POOL_SIGNATURE);

>From the ISO C99 standard, "6.7.2.2 Enumeration specifiers":

      Constraints

    2 The expression that defines the value of an enumeration constant
      shall be an integer constant expression that has a value
      representable as an *int*.

      Semantics

    3 The identifiers in an enumerator list are declared as constants
      that have type *int* and may appear wherever such are
      permitted.^107) An enumerator with = defines its enumeration
      constant as the value of the constant expression. If the first
      enumerator has no =, the value of its enumeration constant is 0.
      Each subsequent enumerator with no = defines its enumeration
      constant as the value of the constant expression obtained by
      adding 1 to the value of the previous enumeration constant. (The
      use of enumerators with = may produce enumeration constants with
      values that duplicate other values in the same enumeration.) The
      enumerators of an enumeration are also known as its members.

    4 Each enumerated type shall be compatible with *char*, a signed
      integer type, or an unsigned integer type. The choice of type is
      implementation-defined,^108) but shall be capable of representing
      the values of all the members of the enumeration. The enumerated
      type is incomplete until after the } that terminates the list of
      enumerator declarations.

    107) Thus, the identifiers of enumeration constants declared in the
         same scope shall all be distinct from each other and from other
         identifiers declared in ordinary declarators.

    108) An implementation may delay the choice of which integer type
         until all enumeration constants have been seen.

As far as I can see from the enumerators in the current typedef, a
compiler implementation could even choose an 8-bit or a 16-bit integer
type for EFI_MEMORY_TYPE. Which would mean that values in
0x80000000..0xFFFFFFFF are not representable at all.

If it's guaranteed that (a) each supported compiler will choose either
int32_t or uint32_t, then

> + if ((INT32)MemoryType < 0) {

is correct, although (b) it may require further implementation-defined
behavior. "6.3.1.3 Signed and unsigned integers":

    3 Otherwise, the new type is signed and the value cannot be
      represented in it; either the result is implementation-defined or
      an implementation-defined signal is raised.

If one wants to rely on (a) only, then

  (UINT32)MemoryType >= 0x80000000u

could be a better controlling expression. (Putting just "0x80000000" on
the RHS, ie. without the "u" suffix, would work as well.)

Laszlo

------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and 
threat landscape has changed and how IT managers can respond. Discussions 
will include endpoint security, mobile security and the latest in malware 
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel