* Re: [PATCH 2/2] OvmfPkg/PlatformPei: sync AmdSevInitialize() definition with declaration
[not found] ` <20180206220115.29193-3-lersek@redhat.com>
@ 2018-02-06 22:11 ` Brijesh Singh
2018-02-07 9:47 ` Ard Biesheuvel
1 sibling, 0 replies; 3+ messages in thread
From: Brijesh Singh @ 2018-02-06 22:11 UTC (permalink / raw)
To: Laszlo Ersek, edk2-devel-01; +Cc: brijesh.singh, Ard Biesheuvel, Jordan Justen
On 02/06/2018 04:01 PM, Laszlo Ersek wrote:
> "Platform.h" declares the AmdSevInitialize() function without EFIAPI, but
> the definition in "AmdSev.c" includes EFIAPI.
>
> GCC toolchains without LTO do not catch this error because "AmdSev.c" does
> not include "Platform.h"; i.e. the declaration used by callers such as
> "Platform.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.
>
> There's been no functional breakage because AmdSevInitialize() takes no
> parameters.
>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Brijesh Singh <brijesh.singh@amd.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Brijesh Singh <brijesh.singh@amd.com>
Thanks
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH 0/2] sync some function definitions with their declarations
[not found] <20180206220115.29193-1-lersek@redhat.com>
@ 2018-02-06 22:56 ` Carsey, Jaben
[not found] ` <20180206220115.29193-3-lersek@redhat.com>
1 sibling, 0 replies; 3+ messages in thread
From: Carsey, Jaben @ 2018-02-06 22:56 UTC (permalink / raw)
To: Laszlo Ersek
Cc: edk2-devel-01, Ard Biesheuvel, Brijesh Singh, Justen, Jordan L,
Ni, Ruiyu
Reviewed-by Jaben Carsey <Jaben.carsey@intel.com>
> On Feb 6, 2018, at 2:01 PM, Laszlo Ersek <lersek@redhat.com> wrote:
>
> Repo: https://github.com/lersek/edk2.git
> Branch: sync_defs_to_decls
>
> The patches say it all, just a side remark up here:
>
> These errors would have been caught long ago if we had enabled the
> "-Wmissing-prototypes" gcc option.
>
> (Unfortunately, we can't enable that option even now, because it --
> laudably -- forces the programmer to give internal linkage ("STATIC") to
> their helper / local functions, and -- as we've learned -- some versions
> of the the Visual Studio debugger choke on STATIC functions. Thus, core
> modules basically never make their internal functions STATIC.)
>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Brijesh Singh <brijesh.singh@amd.com>
> Cc: Jaben Carsey <jaben.carsey@intel.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
>
> Thanks
> Laszlo
>
> Laszlo Ersek (2):
> ShellPkg/UefiShellDebug1CommandsLib: sync Compress() definition with
> decl.
> OvmfPkg/PlatformPei: sync AmdSevInitialize() definition with
> declaration
>
> OvmfPkg/PlatformPei/AmdSev.c | 3 ++-
> ShellPkg/Library/UefiShellDebug1CommandsLib/Compress.c | 15 +++++++++------
> 2 files changed, 11 insertions(+), 7 deletions(-)
>
> --
> 2.14.1.3.gb7cf6e02401b
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH 2/2] OvmfPkg/PlatformPei: sync AmdSevInitialize() definition with declaration
[not found] ` <20180206220115.29193-3-lersek@redhat.com>
2018-02-06 22:11 ` [PATCH 2/2] OvmfPkg/PlatformPei: sync AmdSevInitialize() definition with declaration Brijesh Singh
@ 2018-02-07 9:47 ` Ard Biesheuvel
1 sibling, 0 replies; 3+ messages in thread
From: Ard Biesheuvel @ 2018-02-07 9:47 UTC (permalink / raw)
To: Laszlo Ersek; +Cc: edk2-devel-01, Brijesh Singh, Jordan Justen
On 6 February 2018 at 23:01, Laszlo Ersek <lersek@redhat.com> wrote:
> "Platform.h" declares the AmdSevInitialize() function without EFIAPI, but
> the definition in "AmdSev.c" includes EFIAPI.
>
> GCC toolchains without LTO do not catch this error because "AmdSev.c" does
> not include "Platform.h"; i.e. the declaration used by callers such as
> "Platform.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.
>
> There's been no functional breakage because AmdSevInitialize() takes no
> parameters.
>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Brijesh Singh <brijesh.singh@amd.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> OvmfPkg/PlatformPei/AmdSev.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/OvmfPkg/PlatformPei/AmdSev.c b/OvmfPkg/PlatformPei/AmdSev.c
> index 1539e5b5cdce..ad31b69fb032 100644
> --- a/OvmfPkg/PlatformPei/AmdSev.c
> +++ b/OvmfPkg/PlatformPei/AmdSev.c
> @@ -23,6 +23,8 @@
> #include <Register/Amd/Cpuid.h>
> #include <Library/MemEncryptSevLib.h>
>
> +#include "Platform.h"
> +
> /**
>
> Function checks if SEV support is available, if present then it sets
> @@ -30,7 +32,6 @@
>
> **/
> VOID
> -EFIAPI
> AmdSevInitialize (
> VOID
> )
> --
> 2.14.1.3.gb7cf6e02401b
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-02-07 9:41 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20180206220115.29193-1-lersek@redhat.com>
2018-02-06 22:56 ` [PATCH 0/2] sync some function definitions with their declarations Carsey, Jaben
[not found] ` <20180206220115.29193-3-lersek@redhat.com>
2018-02-06 22:11 ` [PATCH 2/2] OvmfPkg/PlatformPei: sync AmdSevInitialize() definition with declaration Brijesh Singh
2018-02-07 9:47 ` Ard Biesheuvel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox