Re: [PATCH] MdePkg: resolve bug 741

[Marvin H?user <Marvin.Haeuser@outlook.com>]

For code style purposes, I suggest to declare the first member of the language list as a third argument, process it, and then use it to get the following from the VA.
Also, I think you forgot to CC the MdePkg maintainers, so I did with this mail.

Regards,
Marvin.

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Zenith432
> Sent: Saturday, December 9, 2017 8:44 PM
> 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