edk2: clean up the usage of standardized (VOID*) typedefs

[edk2-devel] [PATCH 24/35] OvmfPkg/PlatformDxe: fix EFI_HII_HANDLE parameters of internal functions

Posted by Laszlo Ersek 6 years, 4 months ago

In the following call tree:

 PlatformInit ()
   mInstalledPackages = HiiAddPackages ()
 GopInstalled ()
    PopulateForm (PackageList = mInstalledPackages)
      CreateResolutionOptions (PackageList)
        HiiSetString (PackageList
      HiiUpdateForm (PackageList)

PlatformDxe passes around an EFI_HII_HANDLE that (a) originates from
HiiAddPackages() and (b) is ultimately passed to HiiSetString() and
HiiUpdateForm(). The intermediate functions PopulateForm() and
CreateResolutionOptions() however take that parameter as an
(EFI_HII_HANDLE*).

There is no bug in practice (because the affected functions never try to
de-reference the "PackageList" parameter, they just pass it on), but the
function prototypes are semantically wrong. Fix that.

This could remain hidden so long because pointer-to-VOID silently converts
to/from any pointer-to-object type, and the UEFI spec mandates that
EFI_HII_HANDLE be a typedef to (VOID*).

Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---

Notes:
    tested in UiApp

 OvmfPkg/PlatformDxe/Platform.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/OvmfPkg/PlatformDxe/Platform.c b/OvmfPkg/PlatformDxe/Platform.c
index 09181769babf..23ad43901f66 100644
--- a/OvmfPkg/PlatformDxe/Platform.c
+++ b/OvmfPkg/PlatformDxe/Platform.c
@@ -486,7 +486,7 @@ STATIC
 EFI_STATUS
 EFIAPI
 CreateResolutionOptions (
-  IN  EFI_HII_HANDLE  *PackageList,
+  IN  EFI_HII_HANDLE  PackageList,
   OUT VOID            **OpCodeBuffer,
   IN  UINTN           NumGopModes,
   IN  GOP_MODE        *GopModes
@@ -547,7 +547,7 @@ STATIC
 EFI_STATUS
 EFIAPI
 PopulateForm (
-  IN  EFI_HII_HANDLE  *PackageList,
+  IN  EFI_HII_HANDLE  PackageList,
   IN  EFI_GUID        *FormSetGuid,
   IN  EFI_FORM_ID     FormId,
   IN  UINTN           NumGopModes,
-- 
2.19.1.3.g30247aa5d201



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#47411): https://edk2.groups.io/g/devel/message/47411
Mute This Topic: https://groups.io/mt/34180225/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 24/35] OvmfPkg/PlatformDxe: fix EFI_HII_HANDLE parameters of internal functions

Posted by Ard Biesheuvel 6 years, 4 months ago

On Tue, 17 Sep 2019 at 21:50, Laszlo Ersek <lersek@redhat.com> wrote:
>
> In the following call tree:
>
>  PlatformInit ()
>    mInstalledPackages = HiiAddPackages ()
>  GopInstalled ()
>     PopulateForm (PackageList = mInstalledPackages)
>       CreateResolutionOptions (PackageList)
>         HiiSetString (PackageList
>       HiiUpdateForm (PackageList)
>
> PlatformDxe passes around an EFI_HII_HANDLE that (a) originates from
> HiiAddPackages() and (b) is ultimately passed to HiiSetString() and
> HiiUpdateForm(). The intermediate functions PopulateForm() and
> CreateResolutionOptions() however take that parameter as an
> (EFI_HII_HANDLE*).
>
> There is no bug in practice (because the affected functions never try to
> de-reference the "PackageList" parameter, they just pass it on), but the
> function prototypes are semantically wrong. Fix that.
>
> This could remain hidden so long because pointer-to-VOID silently converts
> to/from any pointer-to-object type, and the UEFI spec mandates that
> EFI_HII_HANDLE be a typedef to (VOID*).
>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>

Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> ---
>
> Notes:
>     tested in UiApp
>
>  OvmfPkg/PlatformDxe/Platform.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/OvmfPkg/PlatformDxe/Platform.c b/OvmfPkg/PlatformDxe/Platform.c
> index 09181769babf..23ad43901f66 100644
> --- a/OvmfPkg/PlatformDxe/Platform.c
> +++ b/OvmfPkg/PlatformDxe/Platform.c
> @@ -486,7 +486,7 @@ STATIC
>  EFI_STATUS
>  EFIAPI
>  CreateResolutionOptions (
> -  IN  EFI_HII_HANDLE  *PackageList,
> +  IN  EFI_HII_HANDLE  PackageList,
>    OUT VOID            **OpCodeBuffer,
>    IN  UINTN           NumGopModes,
>    IN  GOP_MODE        *GopModes
> @@ -547,7 +547,7 @@ STATIC
>  EFI_STATUS
>  EFIAPI
>  PopulateForm (
> -  IN  EFI_HII_HANDLE  *PackageList,
> +  IN  EFI_HII_HANDLE  PackageList,
>    IN  EFI_GUID        *FormSetGuid,
>    IN  EFI_FORM_ID     FormId,
>    IN  UINTN           NumGopModes,
> --
> 2.19.1.3.g30247aa5d201
>
>

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#48040): https://edk2.groups.io/g/devel/message/48040
Mute This Topic: https://groups.io/mt/34180225/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 24/35] OvmfPkg/PlatformDxe: fix EFI_HII_HANDLE parameters of internal functions

Posted by Philippe Mathieu-Daudé 6 years, 4 months ago

On 9/17/19 9:49 PM, Laszlo Ersek wrote:
> In the following call tree:
> 
>  PlatformInit ()
>    mInstalledPackages = HiiAddPackages ()
>  GopInstalled ()
>     PopulateForm (PackageList = mInstalledPackages)
>       CreateResolutionOptions (PackageList)
>         HiiSetString (PackageList
>       HiiUpdateForm (PackageList)
> 
> PlatformDxe passes around an EFI_HII_HANDLE that (a) originates from
> HiiAddPackages() and (b) is ultimately passed to HiiSetString() and
> HiiUpdateForm(). The intermediate functions PopulateForm() and
> CreateResolutionOptions() however take that parameter as an
> (EFI_HII_HANDLE*).
> 
> There is no bug in practice (because the affected functions never try to
> de-reference the "PackageList" parameter, they just pass it on), but the
> function prototypes are semantically wrong. Fix that.
> 
> This could remain hidden so long because pointer-to-VOID silently converts
> to/from any pointer-to-object type, and the UEFI spec mandates that
> EFI_HII_HANDLE be a typedef to (VOID*).
> 
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>

Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>

> ---
> 
> Notes:
>     tested in UiApp
> 
>  OvmfPkg/PlatformDxe/Platform.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/OvmfPkg/PlatformDxe/Platform.c b/OvmfPkg/PlatformDxe/Platform.c
> index 09181769babf..23ad43901f66 100644
> --- a/OvmfPkg/PlatformDxe/Platform.c
> +++ b/OvmfPkg/PlatformDxe/Platform.c
> @@ -486,7 +486,7 @@ STATIC
>  EFI_STATUS
>  EFIAPI
>  CreateResolutionOptions (
> -  IN  EFI_HII_HANDLE  *PackageList,
> +  IN  EFI_HII_HANDLE  PackageList,
>    OUT VOID            **OpCodeBuffer,
>    IN  UINTN           NumGopModes,
>    IN  GOP_MODE        *GopModes
> @@ -547,7 +547,7 @@ STATIC
>  EFI_STATUS
>  EFIAPI
>  PopulateForm (
> -  IN  EFI_HII_HANDLE  *PackageList,
> +  IN  EFI_HII_HANDLE  PackageList,
>    IN  EFI_GUID        *FormSetGuid,
>    IN  EFI_FORM_ID     FormId,
>    IN  UINTN           NumGopModes,
> 

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#47722): https://edk2.groups.io/g/devel/message/47722
Mute This Topic: https://groups.io/mt/34180225/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-