Re: [PATCH] MdePkg: resolve bug 741

["Gao, Liming" <liming.gao@intel.com>]

For 4, 'unsigned char' goes default argument promotion to int. This is CLANG compiler behavior. Does GCC and VS compiler follow this rule?

Disable varargs warning is the temp solution. For long term, we expect to figure out the compatible solution. If all supported compilers follow this rule, I think this is a compatible change. 

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Zenith432
> Sent: Sunday, December 10, 2017 3:44 AM
> To: edk2-devel@lists.01.org
> Subject: [edk2] [PATCH] MdePkg: resolve bug 741
> 
> This is a proposal to resolve bug 741: UefiLib.c compilation failure with clang
> error: passing an object that undergoes default argument promotion to 'va_start' has undefined behavior [-Werror,-Wvarargs]"
> 
> Rationale:
> 1. Default argument promotion causes the sizeof a function's argument to be different than the corresponding
> parameter's sizeof.  This may break a permissible implemenation of stdarg.h, which is why it is considered
> undefined behavior in C.  The condition should be repaired rather than silenced with -Wno-varargs.
> 2. The warning is due to the last non-variadic parameter of GetBestLanguage having type BOOLEAN.
> 3. BOOLEAN is typedef'd to 'unsigned char' in all ProcessorBind.h.
> 4. 'unsigned char' goes default argument promotion to int.
> 5. All ProcessorBind.h typedef the type INT32 to either 'int' or some 32-bit equivalent.
> 6. As a result it is safe to replace the type of the parameter from BOOLEAN to INT32 in all
> current supported architectures.
> 7. The change is consistent with the BOOLEAN argument being converted to INT32 at the caller site. The
> function GetBestLanguage currently converts the argument from INT32 back to BOOLEAN, however
> the function's logic is not disturbed by treating the argument as an INT32.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Zenith432 <zenith432@users.sourceforge.net>
> ---
>  MdePkg/Include/Library/UefiLib.h | 2 +-
>  MdePkg/Library/UefiLib/UefiLib.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/MdePkg/Include/Library/UefiLib.h b/MdePkg/Include/Library/UefiLib.h
> index 0b14792a..5d98eb26 100644
> --- a/MdePkg/Include/Library/UefiLib.h
> +++ b/MdePkg/Include/Library/UefiLib.h
> @@ -818,7 +818,7 @@ CHAR8 *
>  EFIAPI
>  GetBestLanguage (
>    IN CONST CHAR8  *SupportedLanguages,
> -  IN BOOLEAN      Iso639Language,
> +  IN INT32        Iso639Language,
>    ...
>    );
> 
> diff --git a/MdePkg/Library/UefiLib/UefiLib.c b/MdePkg/Library/UefiLib/UefiLib.c
> index a7eee012..57236511 100644
> --- a/MdePkg/Library/UefiLib/UefiLib.c
> +++ b/MdePkg/Library/UefiLib/UefiLib.c
> @@ -1514,7 +1514,7 @@ CHAR8 *
>  EFIAPI
>  GetBestLanguage (
>    IN CONST CHAR8  *SupportedLanguages,
> -  IN BOOLEAN      Iso639Language,
> +  IN INT32        Iso639Language,
>    ...
>    )
>  {
> --
> 2.14.3 (Apple Git-98)
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel