Re: [patch 1/3] ShellPkg/Dp: make sure memory is freed before exit

["Ni, Ruiyu" <ruiyu.ni@intel.com>]

Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>

Thanks/Ray

> -----Original Message-----
> From: Bi, Dandan
> Sent: Tuesday, June 5, 2018 4:33 PM
> To: edk2-devel@lists.01.org
> Cc: Gao, Liming <liming.gao@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>;
> Carsey, Jaben <jaben.carsey@intel.com>
> Subject: [patch 1/3] ShellPkg/Dp: make sure memory is freed before exit
> 
> Run dp command now:
> Firstly it will get performance records from FPDT and then parse the DP
> command. And if encounter invalid parameters, it will exit directly. Thus the
> performance records got before are invalid. And what's worse is that the
> memory allocated in getting performance records phase is not freed.
> 
> This patch update the code to parse the command firstly and then get the
> performance records. And make sure that all the clean work has been done
> before exiting.
> 
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Jaben Carsey <jaben.carsey@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Dandan Bi <dandan.bi@intel.com>
> ---
>  ShellPkg/DynamicCommand/DpDynamicCommand/Dp.c | 70
> ++++++++++++++-------------
>  1 file changed, 37 insertions(+), 33 deletions(-)
> 
> diff --git a/ShellPkg/DynamicCommand/DpDynamicCommand/Dp.c
> b/ShellPkg/DynamicCommand/DpDynamicCommand/Dp.c
> index aa9c2cdf7a8..fe85937f557 100644
> --- a/ShellPkg/DynamicCommand/DpDynamicCommand/Dp.c
> +++ b/ShellPkg/DynamicCommand/DpDynamicCommand/Dp.c
> @@ -390,11 +390,11 @@ BuildCachedGuidHandleTable (
>    }
>    if (HandleBuffer != NULL) {
>      FreePool (HandleBuffer);
>      HandleBuffer = NULL;
>    }
> -  return Status;
> +  return EFI_SUCCESS;
>  }
> 
>  /**
>    Get Measurement form Fpdt records.
> 
> @@ -729,39 +729,10 @@ RunDp (
>    // initialize the shell lib (we must be in non-auto-init...)
>    //
>    Status = ShellInitialize();
>    ASSERT_EFI_ERROR(Status);
> 
> -  //
> -  // DP dump performance data by parsing FPDT table in ACPI table.
> -  // Folloing 3 steps are to get the measurement form the FPDT table.
> -  //
> -
> -  //
> -  //1. Get FPDT from ACPI table.
> -  //
> -  Status = GetBootPerformanceTable ();
> -  if (EFI_ERROR(Status)) {
> -    return Status;
> -  }
> -
> -  //
> -  //2. Cache the ModuleGuid and hanlde mapping table.
> -  //
> -  Status = BuildCachedGuidHandleTable();
> -  if (EFI_ERROR (Status)) {
> -    return Status;
> -  }
> -
> -  //
> -  //3. Build the measurement array form the FPDT records.
> -  //
> -  Status = BuildMeasurementList ();
> -  if (EFI_ERROR(Status)) {
> -    return Status;
> -  }
> -
>    //
>    // Process Command Line arguments
>    //
>    Status = ShellCommandLineParse (ParamList, &ParamPackage, NULL, TRUE);
>    if (EFI_ERROR(Status)) {
> @@ -809,10 +780,42 @@ RunDp (
>  #if PROFILING_IMPLEMENTED
>      ProfileMode = TRUE;
>  #endif  // PROFILING_IMPLEMENTED
>    }
> 
> +  //
> +  // DP dump performance data by parsing FPDT table in ACPI table.
> +  // Folloing 3 steps are to get the measurement form the FPDT table.
> +  //
> +
> +  //
> +  //1. Get FPDT from ACPI table.
> +  //
> +  Status = GetBootPerformanceTable ();
> +  if (EFI_ERROR (Status)) {
> +    ShellStatus = Status;
> +    goto Done;
> +  }
> +
> +  //
> +  //2. Cache the ModuleGuid and hanlde mapping table.
> +  //
> +  Status = BuildCachedGuidHandleTable();  if (EFI_ERROR (Status)) {
> +    ShellStatus = Status;
> +    goto Done;
> +  }
> +
> +  //
> +  //3. Build the measurement array form the FPDT records.
> +  //
> +  Status = BuildMeasurementList ();
> +  if (EFI_ERROR (Status)) {
> +    ShellStatus = SHELL_OUT_OF_RESOURCES;
> +    goto Done;
> +  }
> +
>    //
>    // Initialize the pre-defined cumulative data.
>    //
>    InitCumulativeData ();
> 
> @@ -821,21 +824,22 @@ RunDp (
>    //
>    CustomCumulativeToken = ShellCommandLineGetValue (ParamPackage,
> L"-c");
>    if (CustomCumulativeToken != NULL) {
>      CustomCumulativeData = AllocateZeroPool (sizeof (PERF_CUM_DATA));
>      if (CustomCumulativeData == NULL) {
> -      return SHELL_OUT_OF_RESOURCES;
> +      ShellStatus = SHELL_OUT_OF_RESOURCES;
> +      goto Done;
>      }
>      CustomCumulativeData->MinDur = PERF_MAXDUR;
>      CustomCumulativeData->MaxDur = 0;
>      CustomCumulativeData->Count  = 0;
>      CustomCumulativeData->Duration = 0;
>      NameSize = StrLen (CustomCumulativeToken) + 1;
>      CustomCumulativeData->Name   = AllocateZeroPool (NameSize);
>      if (CustomCumulativeData->Name == NULL) {
> -      FreePool (CustomCumulativeData);
> -      return SHELL_OUT_OF_RESOURCES;
> +      ShellStatus = SHELL_OUT_OF_RESOURCES;
> +      goto Done;
>      }
>      UnicodeStrToAsciiStrS (CustomCumulativeToken, CustomCumulativeData-
> >Name, NameSize);
>    }
> 
>    //
> --
> 2.14.3.windows.1