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

["Bi, Dandan" <dandan.bi@intel.com>]

This patch is also available at
 https://github.com/dandanbi/edk2/commit/e5c432b2d5083c0bbadbf773d460a9c22d36b267


Thanks,
Dandan
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Dandan Bi
> Sent: Thursday, February 14, 2019 10:39 AM
> To: edk2-devel@lists.01.org
> Cc: Wu, Hao A <hao.a.wu@intel.com>; Michael Turner
> <Michael.Turner@microsoft.com>; Max Knutsen
> <maknutse@microsoft.com>; Gao, Liming <liming.gao@intel.com>
> Subject: [edk2] [patch V2] MdeModulePkg/ReportStatusCodeLib: Avoid
> using AllocatePool if possible
> 
> From: Max Knutsen <maknutse@microsoft.com>
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1114
> 
> V2:  simplify the code logic.
> update
> if (!mHaveExitedBootServices &&
>   (StatusCodeData != (EFI_STATUS_CODE_DATA *) StatusCodeBuffer)) {
>   gBS->FreePool (StatusCodeData);
> }
> to
> if (StatusCodeData != (EFI_STATUS_CODE_DATA *) StatusCodeBuffer) {
>   gBS->FreePool (StatusCodeData);
> }
> 
> 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/ReportStatusC
> odeLib.c
> b/MdeModulePkg/Library/RuntimeDxeReportStatusCodeLib/ReportStatusC
> odeLib.c
> index b73103517a..75e2f88ad2 100644
> ---
> a/MdeModulePkg/Library/RuntimeDxeReportStatusCodeLib/ReportStatusC
> odeLib.c
> +++
> b/MdeModulePkg/Library/RuntimeDxeReportStatusCodeLib/ReportStatusC
> od
> +++ eLib.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 (StatusCodeData != (EFI_STATUS_CODE_DATA *) StatusCodeBuffer) {
>      gBS->FreePool (StatusCodeData);
>    }
> 
>    return Status;
>  }
> --
> 2.18.0.windows.1
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel