Re: [patch] MdeModulePkg/ReportStatusCodeLib: Avoid using AllocatePool if possible

["Gao, Liming" <liming.gao@intel.com>]

Dandan:
 The last change in RuntimeDxeReportStatusCodeLib can be simplified. Other change is good. 

  if (!mHaveExitedBootServices && (StatusCodeData != (EFI_STATUS_CODE_DATA *) StatusCodeBuffer)) {
    gBS->FreePool (StatusCodeData);
  }
==>
  if (StatusCodeData != (EFI_STATUS_CODE_DATA *) StatusCodeBuffer) {
    gBS->FreePool (StatusCodeData);
  }

Thanks
Liming
> -----Original Message-----
> From: Bi, Dandan
> Sent: Saturday, February 2, 2019 4:47 PM
> To: edk2-devel@lists.01.org
> Cc: Max Knutsen <maknutse@microsoft.com>; Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Michael Turner
> <Michael.Turner@microsoft.com>; Gao, Liming <liming.gao@intel.com>
> Subject: [patch] MdeModulePkg/ReportStatusCodeLib: Avoid using AllocatePool if possible
> 
> From: Max Knutsen <maknutse@microsoft.com>
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1114
> 
> When report status code with ExtendedData data,
> and the extended data can fit in the local static buffer,
> there is no need to use AllocatePool to hold the ExtendedData data.
> 
> This patch is just to do the enhancement to avoid using AllocatePool.
> 
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Hao Wu <hao.a.wu@intel.com>
> Cc: Michael Turner <Michael.Turner@microsoft.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Dandan Bi <dandan.bi@intel.com>
> ---
>  .../Library/DxeReportStatusCodeLib/ReportStatusCodeLib.c | 9 +++++++--
>  .../RuntimeDxeReportStatusCodeLib/ReportStatusCodeLib.c  | 9 +++++++--
>  2 files changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/MdeModulePkg/Library/DxeReportStatusCodeLib/ReportStatusCodeLib.c
> b/MdeModulePkg/Library/DxeReportStatusCodeLib/ReportStatusCodeLib.c
> index b28dc5c3bb..c88be5a1a3 100644
> --- a/MdeModulePkg/Library/DxeReportStatusCodeLib/ReportStatusCodeLib.c
> +++ b/MdeModulePkg/Library/DxeReportStatusCodeLib/ReportStatusCodeLib.c
> @@ -505,13 +505,18 @@ ReportStatusCodeEx (
>    //
>    Tpl = gBS->RaiseTPL (TPL_HIGH_LEVEL);
>    gBS->RestoreTPL (Tpl);
> 
>    StatusCodeData = NULL;
> -  if (Tpl <= TPL_NOTIFY) {
> +  if (ExtendedDataSize <= (MAX_EXTENDED_DATA_SIZE - sizeof (EFI_STATUS_CODE_DATA))) {
>      //
> -    // Allocate space for the Status Code Header and its buffer
> +    // Use Buffer instead of allocating if possible.
> +    //
> +    StatusCodeData = (EFI_STATUS_CODE_DATA *)Buffer;
> +  } else if (Tpl <= TPL_NOTIFY){
> +    //
> +    // If Buffer is not big enough, allocate space for the Status Code Header and its buffer
>      //
>      gBS->AllocatePool (EfiBootServicesData, sizeof (EFI_STATUS_CODE_DATA) + ExtendedDataSize, (VOID **)&StatusCodeData);
>    }
> 
>    if (StatusCodeData == NULL) {
> diff --git a/MdeModulePkg/Library/RuntimeDxeReportStatusCodeLib/ReportStatusCodeLib.c
> b/MdeModulePkg/Library/RuntimeDxeReportStatusCodeLib/ReportStatusCodeLib.c
> index b73103517a..3380469759 100644
> --- a/MdeModulePkg/Library/RuntimeDxeReportStatusCodeLib/ReportStatusCodeLib.c
> +++ b/MdeModulePkg/Library/RuntimeDxeReportStatusCodeLib/ReportStatusCodeLib.c
> @@ -635,11 +635,14 @@ ReportStatusCodeEx (
>    if (mHaveExitedBootServices) {
>      if (sizeof (EFI_STATUS_CODE_DATA) + ExtendedDataSize > MAX_EXTENDED_DATA_SIZE) {
>        return EFI_OUT_OF_RESOURCES;
>      }
>      StatusCodeData = (EFI_STATUS_CODE_DATA *) StatusCodeBuffer;
> -  } else  {
> +  } else if (sizeof (EFI_STATUS_CODE_DATA) + ExtendedDataSize > MAX_EXTENDED_DATA_SIZE) {
> +    //
> +    // Only use AllocatePool for larger ExtendedData.
> +    //
>      if (gBS == NULL || gBS->AllocatePool == NULL || gBS->FreePool == NULL) {
>        return EFI_UNSUPPORTED;
>      }
> 
>      //
> @@ -648,10 +651,12 @@ ReportStatusCodeEx (
>      StatusCodeData = NULL;
>      gBS->AllocatePool (EfiBootServicesData, sizeof (EFI_STATUS_CODE_DATA) + ExtendedDataSize, (VOID **)&StatusCodeData);
>      if (StatusCodeData == NULL) {
>        return EFI_OUT_OF_RESOURCES;
>      }
> +  } else {
> +    StatusCodeData = (EFI_STATUS_CODE_DATA *) StatusCodeBuffer;
>    }
> 
>    //
>    // Fill in the extended data header
>    //
> @@ -678,11 +683,11 @@ ReportStatusCodeEx (
>    Status = InternalReportStatusCode (Type, Value, Instance, CallerId, StatusCodeData);
> 
>    //
>    // Free the allocated buffer
>    //
> -  if (!mHaveExitedBootServices) {
> +  if (!mHaveExitedBootServices && (StatusCodeData != (EFI_STATUS_CODE_DATA *) StatusCodeBuffer)) {
>      gBS->FreePool (StatusCodeData);
>    }
> 
>    return Status;
>  }
> --
> 2.18.0.windows.1