Re: [edk2-devel] [PATCH 10/35] MdeModulePkg/PlatformVarCleanupLib: fix HiiConstructConfigHdr() call

["Philippe Mathieu-Daudé" <philmd@redhat.com>]

On 9/17/19 9:49 PM, Laszlo Ersek wrote:
> The HiiConstructConfigHdr() function takes the "DriverHandle" parameter in
> order to fetch the device path from it, and then turn the device path into
> PATH routing information.
> 
> The HiiConstructConfigHdr() function is called from
> VariableCleanupHiiExtractConfig(), which is only installed when "Type" is
> "VarCleanupManually" in PlatformVarCleanup().
> 
> In that case, we create "Private->DriverHandle" as a new handle, and
> install "mVarCleanupHiiVendorDevicePath" on it. Then we pass
> "Private->DriverHandle" to HiiAddPackages(), which consumes the device
> path for routing purposes.
> 
> It follows that the "DriverHandle" argument pased to

"passed"

> HiiConstructConfigHdr() should be the same driver handle, for matching
> routing.
> 
> Currently we pass "Private->HiiHandle", which is clearly a typo, because
> it is the return value of HiiAddPackages(), and stands for the published
> HII package list.

Phew...

> Therefore this patch addresses an actual bug.
> 
> The typo has not been flagged by compilers because the UEFI spec
> regrettably defines both EFI_HANDLE and EFI_HII_HANDLE as (VOID*).
> 
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
> 
> Notes:
>     build-tested only
> 
>  MdeModulePkg/Library/PlatformVarCleanupLib/PlatVarCleanupLib.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/MdeModulePkg/Library/PlatformVarCleanupLib/PlatVarCleanupLib.c b/MdeModulePkg/Library/PlatformVarCleanupLib/PlatVarCleanupLib.c
> index 968c044a316a..3875d614bb41 100644
> --- a/MdeModulePkg/Library/PlatformVarCleanupLib/PlatVarCleanupLib.c
> +++ b/MdeModulePkg/Library/PlatformVarCleanupLib/PlatVarCleanupLib.c
> @@ -609,7 +609,11 @@ VariableCleanupHiiExtractConfig (
>      // Allocate and fill a buffer large enough to hold the <ConfigHdr> template
>      // followed by "&OFFSET=0&WIDTH=WWWWWWWWWWWWWWWW" followed by a Null-terminator.
>      //
> -    ConfigRequestHdr = HiiConstructConfigHdr (&mVariableCleanupHiiGuid, mVarStoreName, Private->HiiHandle);
> +    ConfigRequestHdr = HiiConstructConfigHdr (
> +                         &mVariableCleanupHiiGuid,
> +                         mVarStoreName,
> +                         Private->DriverHandle
> +                         );
>      Size = (StrLen (ConfigRequestHdr) + 32 + 1) * sizeof (CHAR16);
>      ConfigRequest = AllocateZeroPool (Size);
>      ASSERT (ConfigRequest != NULL);
> 

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