1. Patchew
  2. EDK2
  3. [edk2] [PATCH v2 0/2] sync some function definitions with their declarations
  4. View patch

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

Laszlo Ersek posted 2 patches 6 years, 10 months ago
[edk2] [PATCH v2 1/2] ShellPkg/UefiShellDebug1CommandsLib: sync Compress() definition with decl.
Posted by Laszlo Ersek 6 years, 10 months ago 
"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.)

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
Re: [edk2] [PATCH v2 1/2] ShellPkg/UefiShellDebug1CommandsLib: sync Compress() definition with decl.
Posted by Gary Lin 6 years, 10 months ago 
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
> 
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Patchew 2.1-devel-8c64711

© 2016 - 2024 Red Hat, Inc.