From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=195.135.221.5; helo=smtp.nue.novell.com; envelope-from=glin@suse.com; receiver=edk2-devel@lists.01.org Received: from smtp.nue.novell.com (smtp.nue.novell.com [195.135.221.5]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 7BDAE22361E63 for ; Wed, 7 Feb 2018 19:20:03 -0800 (PST) Received: from emea4-mta.ukb.novell.com ([10.120.13.87]) by smtp.nue.novell.com with ESMTP (TLS encrypted); Thu, 08 Feb 2018 04:25:43 +0100 Received: from GaryWorkstation (nwb-a10-snat.microfocus.com [10.120.13.202]) by emea4-mta.ukb.novell.com with ESMTP (TLS encrypted); Thu, 08 Feb 2018 03:25:36 +0000 Date: Thu, 8 Feb 2018 11:25:31 +0800 From: Gary Lin To: Laszlo Ersek Cc: edk2-devel@lists.01.org, Jaben Carsey , Ruiyu Ni Message-ID: <20180208032531.wew44qup62nhdgtd@GaryWorkstation> References: <20180207224435.10730-1-lersek@redhat.com> <20180207224435.10730-2-lersek@redhat.com> MIME-Version: 1.0 In-Reply-To: <20180207224435.10730-2-lersek@redhat.com> User-Agent: NeoMutt/20170912 (1.9.0) Subject: Re: [PATCH v2 1/2] ShellPkg/UefiShellDebug1CommandsLib: sync Compress() definition with decl. X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 08 Feb 2018 03:20:04 -0000 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline 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 > 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 > Cc: Jaben Carsey > Cc: Ruiyu Ni > Fixes: c4e74e9b814cfb4b51cf832f3bb218cd2aba348b > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Laszlo Ersek > --- > > 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 > #include > > +#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 >