Re: [Patch v4] UefiCpuPkg RegisterCpuFeaturesLib: Fix buffer pointer error usage.

["Kinney, Michael D" <michael.d.kinney@intel.com>]

Eric,

This looks good.

The only minor comment is a code style issue for the parameter
indent in the calls to AllocateCopyPool().  Should look like
the following:

  SupportBitMask = AllocateCopyPool (
                     PcdGetSize (PcdCpuFeaturesSupport),
                     PcdGetPtr (PcdCpuFeaturesSupport)
                     );


With this change,

Reviewed-by: Kinney, Michael D <michael.d.kinney@intel.com>

Mike

> -----Original Message-----
> From: Dong, Eric
> Sent: Tuesday, August 15, 2017 10:21 PM
> To: edk2-devel@lists.01.org
> Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Shao, Ming
> <ming.shao@intel.com>; Kinney; Kinney, Michael D
> <michael.d.kinney@intel.com>
> Subject: [Patch v4] UefiCpuPkg RegisterCpuFeaturesLib: Fix
> buffer pointer error usage.
> 
> Current code allocate buffer for the pointer which later get
> value
> from PCD database. but current code error use "=" for this case.
> Use AllocateCopyPool instead to fix it.
> 
> V2 enhanced to directly use AllocateCopyPool to get the PCD
> value.
> V3 enhanced to avoid using local temp variable.
> V4 enhanced to keep the functions to get the pcd values.
> 
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Shao Ming <ming.shao@intel.com>
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Eric Dong <eric.dong@intel.com>
> ---
>  .../RegisterCpuFeaturesLib/CpuFeaturesInitialize.c       | 16
> ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git
> a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitializ
> e.c
> b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitializ
> e.c
> index 474aea3..b8f76f1 100644
> ---
> a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitializ
> e.c
> +++
> b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitializ
> e.c
> @@ -60,13 +60,13 @@ GetSupportPcds (
>    VOID
>    )
>  {
> -  UINTN                  BitMaskSize;
>    UINT8                  *SupportBitMask;
> 
> -  BitMaskSize = PcdGetSize (PcdCpuFeaturesSupport);
> -  SupportBitMask = AllocateZeroPool (BitMaskSize);
> +  SupportBitMask = AllocateCopyPool (
> +          PcdGetSize (PcdCpuFeaturesSupport),
> +          PcdGetPtr (PcdCpuFeaturesSupport)
> +          );
>    ASSERT (SupportBitMask != NULL);
> -  SupportBitMask = (UINT8 *) PcdGetPtr (PcdCpuFeaturesSupport);
> 
>    return SupportBitMask;
>  }
> @@ -81,13 +81,13 @@ GetConfigurationPcds (
>    VOID
>    )
>  {
> -  UINTN                  BitMaskSize;
>    UINT8                  *SupportBitMask;
> 
> -  BitMaskSize = PcdGetSize (PcdCpuFeaturesUserConfiguration);
> -  SupportBitMask = AllocateZeroPool (BitMaskSize);
> +  SupportBitMask = AllocateCopyPool (
> +          PcdGetSize (PcdCpuFeaturesUserConfiguration),
> +          PcdGetPtr (PcdCpuFeaturesUserConfiguration)
> +          );
>    ASSERT (SupportBitMask != NULL);
> -  SupportBitMask = (UINT8 *) PcdGetPtr
> (PcdCpuFeaturesUserConfiguration);
> 
>    return SupportBitMask;
>  }
> --
> 2.7.0.windows.1