Re: [PATCH v2 1/2] ShellPkg/UefiShellDebug1CommandsLib: sync Compress() definition with decl.

[Gary Lin <glin@suse.com>]

On Wed, Feb 07, 2018 at 11:44:34PM +0100, Laszlo Ersek wrote:
> "Compress.h" declares the Compress() function as EFIAPI, but the
> definition in "Compress.c" lacks EFIAPI.
> 
> GCC toolchains without LTO do not catch this error because "Compress.c"
> does not include "Compress.h"; i.e. the declaration used by callers such
> as "EfiCompress.c" is not actually matched against the function definition
> at build time.
> 
> With LTO enabled, the mismatch is found -- however, as a warning only, due
> to commit f8d0b9662993 ("BaseTools GCC5: disable warnings-as-errors for
> now", 2016-08-03).
> 
> Include the header in the C file (which turns the issue into a hard build
> error on all GCC toolchains), plus sync the declaration from the header
> file to the C file. Finally, remove EFIAPI from both declaration and
> definition -- this was the original intent of commit c4e74e9b814c
> ("ShellPkg/UefiShellDebug1CommandsLib: Remove unnecessary EFIAPI",
> 2016-10-09), but it missed the header file.
> 
> (Gary meant to address that omission in Oct 2017:
> 
>   [edk2] [PATCH] ShellPkg/UefiShellDebug1CommandsLib: Remove EFIAPI from
>                  Compress()
> 
>   http://mid.mail-archive.com/20171026065329.32311-1-glin@suse.com
> 
> and Ray reviewed the patch, but then the patch was never committed.)
> 
I forgot to track the commit. Thanks for picking it up!

Reviewed-by Gary Lin <glin@suse.com>

> So do the sync and drop EFIAPI now.
> 
> This happens to fix the EFICOMPRESS shell command, when built with GCC for
> X64.
> 
> Cc: Gary Lin <glin@suse.com>
> Cc: Jaben Carsey <jaben.carsey@intel.com>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Fixes: c4e74e9b814cfb4b51cf832f3bb218cd2aba348b
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
> 
> Notes:
>     v2:
>     - drop EFIAPI based on Gary's earlier patch which I just stumbled upon
>     - do not pick up Jaben's R-b from v1, due to inverted EFIAPI handling
>     - add "Fixes:" tag
> 
>  ShellPkg/Library/UefiShellDebug1CommandsLib/Compress.h |  1 -
>  ShellPkg/Library/UefiShellDebug1CommandsLib/Compress.c | 14 ++++++++------
>  2 files changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/ShellPkg/Library/UefiShellDebug1CommandsLib/Compress.h b/ShellPkg/Library/UefiShellDebug1CommandsLib/Compress.h
> index 39a997178fb7..7fe844e212a8 100644
> --- a/ShellPkg/Library/UefiShellDebug1CommandsLib/Compress.h
> +++ b/ShellPkg/Library/UefiShellDebug1CommandsLib/Compress.h
> @@ -28,7 +28,6 @@
>    @retval EFI_BUFFER_TOO_SMALL  The buffer was too small.  DstSize is required.
>  **/
>  EFI_STATUS
> -EFIAPI
>  Compress (
>    IN      VOID    *SrcBuffer,
>    IN      UINT64  SrcSize,
> diff --git a/ShellPkg/Library/UefiShellDebug1CommandsLib/Compress.c b/ShellPkg/Library/UefiShellDebug1CommandsLib/Compress.c
> index 736d2a35b3ac..cde2c54f1b45 100644
> --- a/ShellPkg/Library/UefiShellDebug1CommandsLib/Compress.c
> +++ b/ShellPkg/Library/UefiShellDebug1CommandsLib/Compress.c
> @@ -23,6 +23,8 @@
>  #include <Library/DebugLib.h>
>  #include <Library/ShellLib.h>
>  
> +#include "Compress.h"
> +
>  //
>  // Macro Definitions
>  //
> @@ -1307,20 +1309,20 @@ Encode (
>    The compression routine.
>  
>    @param[in]       SrcBuffer     The buffer containing the source data.
> -  @param[in]       SrcSize       The number of bytes in SrcBuffer.
> +  @param[in]       SrcSize       Number of bytes in SrcBuffer.
>    @param[in]       DstBuffer     The buffer to put the compressed image in.
>    @param[in, out]  DstSize       On input the size (in bytes) of DstBuffer, on
> -                                return the number of bytes placed in DstBuffer.
> +                                 return the number of bytes placed in DstBuffer.
>  
>    @retval EFI_SUCCESS           The compression was sucessful.
>    @retval EFI_BUFFER_TOO_SMALL  The buffer was too small.  DstSize is required.
>  **/
>  EFI_STATUS
>  Compress (
> -  IN       VOID   *SrcBuffer,
> -  IN       UINT64 SrcSize,
> -  IN       VOID   *DstBuffer,
> -  IN OUT   UINT64 *DstSize
> +  IN      VOID    *SrcBuffer,
> +  IN      UINT64  SrcSize,
> +  IN      VOID    *DstBuffer,
> +  IN OUT  UINT64  *DstSize
>    )
>  {
>    EFI_STATUS  Status;
> -- 
> 2.14.1.3.gb7cf6e02401b
> 
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
>